From fa844c6bb40f07d3ead5f6f148cfe619e8e233d5 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Wed, 25 Feb 2026 15:32:20 +0530 Subject: [PATCH 01/32] Support Firewall for public IPs in VPC --- .../orchestration/NetworkOrchestrator.java | 1 + .../upgrade/dao/Upgrade42210to42300.java | 64 +++++++++++++++++++ .../cluster/KubernetesClusterManagerImpl.java | 3 +- .../ConfigurationManagerImpl.java | 2 +- .../element/VpcVirtualRouterElement.java | 4 -- .../network/RoutedIpv4ManagerImpl.java | 3 +- systemvm/debian/opt/cloud/bin/configure.py | 4 +- systemvm/debian/opt/cloud/bin/cs/CsAddress.py | 12 ++++ ui/src/views/network/PublicIpResource.vue | 16 +++-- ui/src/views/offering/AddVpcOffering.vue | 4 +- 10 files changed, 93 insertions(+), 20 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index ce5b3f6d0947..6ef5c7b55e78 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -593,6 +593,7 @@ public boolean configure(final String name, final Map params) th defaultVPCOffProviders.put(Service.StaticNat, defaultProviders); defaultVPCOffProviders.put(Service.PortForwarding, defaultProviders); defaultVPCOffProviders.put(Service.Vpn, defaultProviders); + defaultVPCOffProviders.put(Service.Firewall, defaultProviders); Transaction.execute(new TransactionCallbackNoReturn() { @Override diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42300.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42300.java index 393f20399503..a8e5365e5c10 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42300.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42300.java @@ -17,6 +17,8 @@ package com.cloud.upgrade.dao; import java.io.InputStream; +import java.util.Arrays; +import java.util.List; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -51,6 +53,7 @@ public InputStream[] getPrepareScripts() { @Override public void performDataMigration(Connection conn) { unhideJsInterpretationEnabled(conn); + updateVpcDefaultOfferingsWithFirewallService(conn); } protected void unhideJsInterpretationEnabled(Connection conn) { @@ -89,4 +92,65 @@ protected void updateJsInterpretationEnabledFields(Connection conn, String encry logger.warn("Error while decrypting configuration 'js.interpretation.enabled'. The configuration may already be decrypted."); } } + + private void updateVpcDefaultOfferingsWithFirewallService(Connection conn) { + logger.debug("Updating default VPC offerings to add Firewall service with VpcVirtualRouter provider"); + + final List defaultVpcOfferingUniqueNames = Arrays.asList( + "DefaultIsolatedNetworkOfferingForVpcNetworks", + "DefaultIsolatedNetworkOfferingForVpcNetworksNoLB", + "DefaultIsolatedNetworkOfferingForVpcNetworksWithInternalLB", + "DefaultNATNSXNetworkOfferingForVpc", + "DefaultRoutedNSXNetworkOfferingForVpc", + "DefaultNATNSXNetworkOfferingForVpcWithInternalLB", + "DefaultRoutedNetrisNetworkOfferingForVpc", + "DefaultNATNetrisNetworkOfferingForVpc", + "DefaultNSXVPCNetworkOfferingforKubernetesService" + ); + + try { + for (String uniqueName : defaultVpcOfferingUniqueNames) { + PreparedStatement pstmt = conn.prepareStatement("SELECT id FROM `cloud`.`network_offerings` WHERE unique_name = ?"); + pstmt.setString(1, uniqueName); + + ResultSet rs = pstmt.executeQuery(); + if (!rs.next()) { + continue; + } + + long offeringId = rs.getLong(1); + rs.close(); + pstmt.close(); + + // Insert into ntwk_offering_service_map (if not exists) + pstmt = conn.prepareStatement("INSERT INTO `cloud`.`ntwk_offering_service_map` " + + "(network_offering_id, service, provider, created) " + + "VALUES (?, 'Firewall', 'VpcVirtualRouter', now())"); + pstmt.setLong(1, offeringId); + pstmt.executeUpdate(); + pstmt.close(); + + // Update existing networks (ntwk_service_map) + pstmt = conn.prepareStatement("SELECT id FROM `cloud`.`networks` WHERE network_offering_id = ?"); + pstmt.setLong(1, offeringId); + + rs = pstmt.executeQuery(); + while (rs.next()) { + long networkId = rs.getLong(1); + PreparedStatement insertService = conn.prepareStatement("INSERT INTO `cloud`.`ntwk_service_map` " + + "(network_id, service, provider, created) " + + "VALUES (?, 'Firewall', 'VpcVirtualRouter', now())"); + insertService.setLong(1, networkId); + insertService.executeUpdate(); + insertService.close(); + } + + rs.close(); + pstmt.close(); + } + + } catch (SQLException e) { + logger.warn("Exception while updating VPC default offerings with Firewall service: " + e.getMessage(), e); + } + } } diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java index fea20eb124fc..aa5ddf0cd006 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java @@ -2986,9 +2986,8 @@ private void createNetworkOfferingForKubernetes(String offeringName, String offe defaultKubernetesServiceNetworkOfferingProviders.put(Service.UserData, provider); if (forVpc) { defaultKubernetesServiceNetworkOfferingProviders.put(Service.NetworkACL, forNsx ? Network.Provider.Nsx : provider); - } else { - defaultKubernetesServiceNetworkOfferingProviders.put(Service.Firewall, forNsx ? Network.Provider.Nsx : provider); } + defaultKubernetesServiceNetworkOfferingProviders.put(Service.Firewall, forNsx ? Network.Provider.Nsx : provider); defaultKubernetesServiceNetworkOfferingProviders.put(Service.Lb, forNsx ? Network.Provider.Nsx : provider); defaultKubernetesServiceNetworkOfferingProviders.put(Service.SourceNat, forNsx ? Network.Provider.Nsx : provider); defaultKubernetesServiceNetworkOfferingProviders.put(Service.StaticNat, forNsx ? Network.Provider.Nsx : provider); diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 9f540a590437..d9cdf652c764 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -7263,7 +7263,7 @@ public NetworkOffering createNetworkOffering(final NetworkOfferingBaseCmd cmd) { } if (forVpc == null) { - if (service == Service.SecurityGroup || service == Service.Firewall) { + if (service == Service.SecurityGroup) { forVpc = false; } else if (service == Service.NetworkACL) { forVpc = true; diff --git a/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java b/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java index f393ef8a129d..2b12c1a9f13d 100644 --- a/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java +++ b/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java @@ -412,10 +412,6 @@ private static Map> setCapabilities() { vpnCapabilities.putAll(capabilities.get(Service.Vpn)); vpnCapabilities.put(Capability.VpnTypes, "s2svpn"); capabilities.put(Service.Vpn, vpnCapabilities); - - // remove firewall capability - capabilities.remove(Service.Firewall); - // add network ACL capability final Map networkACLCapabilities = new HashMap(); networkACLCapabilities.put(Capability.SupportedProtocols, "tcp,udp,icmp"); diff --git a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java index a03db4d4a241..fbc5374066e7 100644 --- a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java @@ -989,8 +989,7 @@ public boolean applyRoutingFirewallRule(long id) { @Override public boolean isVirtualRouterGateway(Network network) { return isRoutedNetwork(network) - && (networkServiceMapDao.canProviderSupportServiceInNetwork(network.getId(), Service.Gateway, Provider.VirtualRouter)) - || networkServiceMapDao.canProviderSupportServiceInNetwork(network.getId(), Service.Gateway, Provider.VPCVirtualRouter); + && (networkServiceMapDao.canProviderSupportServiceInNetwork(network.getId(), Service.Gateway, Provider.VirtualRouter)); } @Override diff --git a/systemvm/debian/opt/cloud/bin/configure.py b/systemvm/debian/opt/cloud/bin/configure.py index bf48be66694c..b2c0676c65d0 100755 --- a/systemvm/debian/opt/cloud/bin/configure.py +++ b/systemvm/debian/opt/cloud/bin/configure.py @@ -705,8 +705,8 @@ def process(self): for item in self.dbag: if item == "id": - continue - if self.config.is_vpc(): + continue + if self.config.is_vpc() and not ("purpose" in self.dbag[item] and self.dbag[item]["purpose"] == "Firewall"): self.AclDevice(self.dbag[item], self.config).create() else: self.AclIP(self.dbag[item], self.config).create() diff --git a/systemvm/debian/opt/cloud/bin/cs/CsAddress.py b/systemvm/debian/opt/cloud/bin/cs/CsAddress.py index 37ca8979edc7..c3a4fe40ba1e 100755 --- a/systemvm/debian/opt/cloud/bin/cs/CsAddress.py +++ b/systemvm/debian/opt/cloud/bin/cs/CsAddress.py @@ -647,6 +647,18 @@ def fw_vpcrouter(self): (self.address['network'], self.address['network'])]) if self.get_type() in ["public"]: + # Add PREROUTING firewall chain jump for public IP + self.fw.append(["mangle", "front", + "-A PREROUTING " + + "-d %s/32 -j FIREWALL_%s" % (self.address['public_ip'], self.address['public_ip'])]) + + # Add the firewall chain with default DROP policy + self.fw.append(["mangle", "front", + "-A FIREWALL_%s " % self.address['public_ip'] + + "-m state --state RELATED,ESTABLISHED -j RETURN"]) + self.fw.append(["mangle", "", + "-A FIREWALL_%s -j DROP" % self.address['public_ip']]) + self.fw.append( ["mangle", "", "-A FORWARD -j VPN_STATS_%s" % self.dev]) self.fw.append( diff --git a/ui/src/views/network/PublicIpResource.vue b/ui/src/views/network/PublicIpResource.vue index 0540e7f292a8..46ec6e66bd3c 100644 --- a/ui/src/views/network/PublicIpResource.vue +++ b/ui/src/views/network/PublicIpResource.vue @@ -139,20 +139,22 @@ export default { // VPC IPs with source nat have only VPN when VPC offering conserve mode = false if (this.resource.issourcenat && vpc?.vpcofferingconservemode === false) { - this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'vpn')) + this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => ['vpn', 'firewall'].includes(tab.name))) return } - // VPC IPs with static nat have nothing + // VPC IPs with static nat have firewall if (this.resource.isstaticnat) { if (this.resource.virtualmachinetype === 'DomainRouter') { - this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'vpn')) + this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => ['vpn', 'firewall'].includes(tab.name))) + } else { + this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'firewall')) } return } - // VPC IPs don't have firewall - let tabs = this.$route.meta.tabs.filter(tab => tab.name !== 'firewall') + // VPC IPs now have firewall support + let tabs = this.$route.meta.tabs const network = await this.fetchNetwork() if (network && network.networkofferingconservemode) { @@ -168,12 +170,12 @@ export default { this.portFWRuleCount = await this.fetchPortFWRule() this.loadBalancerRuleCount = await this.fetchLoadBalancerRule() - // VPC IPs with PF only have PF + // VPC IPs with PF only have PF (and firewall) if (this.portFWRuleCount > 0) { tabs = tabs.filter(tab => tab.name !== 'loadbalancing') } - // VPC IPs with LB rules only have LB + // VPC IPs with LB rules only have LB (and firewall) if (this.loadBalancerRuleCount > 0) { tabs = tabs.filter(tab => tab.name !== 'portforwarding') } diff --git a/ui/src/views/offering/AddVpcOffering.vue b/ui/src/views/offering/AddVpcOffering.vue index 1efbfd4df1d6..1c896d9d38dc 100644 --- a/ui/src/views/offering/AddVpcOffering.vue +++ b/ui/src/views/offering/AddVpcOffering.vue @@ -540,7 +540,7 @@ export default { this.supportedServices = [] this.supportedServiceLoading = true getAPI('listSupportedNetworkServices').then(json => { - const vpcServices = ['Dhcp', 'Dns', 'Lb', 'Gateway', 'StaticNat', 'SourceNat', 'NetworkACL', 'PortForwarding', 'UserData', 'Vpn', 'Connectivity', 'CustomAction'] + const vpcServices = ['Dhcp', 'Dns', 'Lb', 'Gateway', 'StaticNat', 'SourceNat', 'NetworkACL', 'PortForwarding', 'UserData', 'Vpn', 'Connectivity', 'CustomAction', 'Firewall'] services = (json?.listsupportednetworkservicesresponse?.networkservice || []) .filter(service => vpcServices.includes(service.name)) .map(service => { @@ -575,7 +575,7 @@ export default { this.supportedServices = [] if (this.networkmode === 'ROUTED') { - services = services.filter(service => !['SourceNat', 'StaticNat', 'Lb', 'PortForwarding', 'Vpn'].includes(service.name)) + services = services.filter(service => !['SourceNat', 'StaticNat', 'Lb', 'PortForwarding', 'Vpn', 'Firewall'].includes(service.name)) } this.supportedServices = services }).catch(error => { From ab71e00b39635c2738322f1a58a40a8900bf8084 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Thu, 5 Mar 2026 11:53:16 +0530 Subject: [PATCH 02/32] UI changes --- .../com/cloud/network/vpc/VpcManagerImpl.java | 2 +- .../network/RoutedIpv4ManagerImpl.java | 3 +-- ui/src/views/network/PublicIpResource.vue | 25 +++++++++++++++---- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index e2083fac7ff8..bcf2c6176efe 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -336,7 +336,7 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis private final ScheduledExecutorService _executor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("VpcChecker")); private List vpcElements = null; - private final List nonSupportedServices = Arrays.asList(Service.SecurityGroup, Service.Firewall); + private final List nonSupportedServices = Arrays.asList(Service.SecurityGroup); private final List supportedProviders = Arrays.asList(Provider.VPCVirtualRouter, Provider.NiciraNvp, Provider.InternalLbVm, Provider.Netscaler, Provider.JuniperContrailVpcRouter, Provider.Ovs, Provider.BigSwitchBcf, Provider.ConfigDrive, Provider.Nsx, Provider.Netris); diff --git a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java index fbc5374066e7..07f6346a8786 100644 --- a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java @@ -995,8 +995,7 @@ public boolean isVirtualRouterGateway(Network network) { @Override public boolean isVirtualRouterGateway(NetworkOffering networkOffering) { return NetworkOffering.NetworkMode.ROUTED.equals(networkOffering.getNetworkMode()) - && networkOfferingServiceMapDao.canProviderSupportServiceInNetworkOffering(networkOffering.getId(), Service.Gateway, Provider.VirtualRouter) - || networkOfferingServiceMapDao.canProviderSupportServiceInNetworkOffering(networkOffering.getId(), Service.Gateway, Provider.VPCVirtualRouter); + && networkOfferingServiceMapDao.canProviderSupportServiceInNetworkOffering(networkOffering.getId(), Service.Gateway, Provider.VirtualRouter); } @Override diff --git a/ui/src/views/network/PublicIpResource.vue b/ui/src/views/network/PublicIpResource.vue index 46ec6e66bd3c..79b0da853d58 100644 --- a/ui/src/views/network/PublicIpResource.vue +++ b/ui/src/views/network/PublicIpResource.vue @@ -139,22 +139,37 @@ export default { // VPC IPs with source nat have only VPN when VPC offering conserve mode = false if (this.resource.issourcenat && vpc?.vpcofferingconservemode === false) { - this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => ['vpn', 'firewall'].includes(tab.name))) + let tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'vpn')) + if (this.resource.associatednetworkid) { + tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => ['vpn', 'firewall'].includes(tab.name))) + } + this.tabs = tabs return } - // VPC IPs with static nat have firewall + // VPC IPs with static nat have firewall (only if associatednetworkid present) if (this.resource.isstaticnat) { if (this.resource.virtualmachinetype === 'DomainRouter') { - this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => ['vpn', 'firewall'].includes(tab.name))) + let tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'vpn')) + if (this.resource.associatednetworkid) { + tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => ['vpn', 'firewall'].includes(tab.name))) + } + this.tabs = tabs } else { - this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'firewall')) + if (this.resource.associatednetworkid) { + this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'firewall')) + } else { + this.tabs = this.defaultTabs + } } return } - // VPC IPs now have firewall support + // VPC IPs have all tabs, but firewall only if associatednetworkid present let tabs = this.$route.meta.tabs + if (!this.resource.associatednetworkid) { + tabs = tabs.filter(tab => tab.name !== 'firewall') + } const network = await this.fetchNetwork() if (network && network.networkofferingconservemode) { From bf97ab8ba2865321ac9a9d09678aaae9f552af4f Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Thu, 5 Mar 2026 15:33:39 +0530 Subject: [PATCH 03/32] more fixes --- .../configuration/ConfigurationManagerImpl.java | 2 ++ ui/src/views/network/PublicIpResource.vue | 17 ++--------------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index d9cdf652c764..e35f97d0b75e 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -7267,6 +7267,8 @@ public NetworkOffering createNetworkOffering(final NetworkOfferingBaseCmd cmd) { forVpc = false; } else if (service == Service.NetworkACL) { forVpc = true; + } else if (service == Service.Firewall) { + forVpc = true; } } diff --git a/ui/src/views/network/PublicIpResource.vue b/ui/src/views/network/PublicIpResource.vue index 79b0da853d58..f92270e576c9 100644 --- a/ui/src/views/network/PublicIpResource.vue +++ b/ui/src/views/network/PublicIpResource.vue @@ -147,29 +147,16 @@ export default { return } - // VPC IPs with static nat have firewall (only if associatednetworkid present) + // VPC IPs with static nat have nothing if (this.resource.isstaticnat) { if (this.resource.virtualmachinetype === 'DomainRouter') { - let tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'vpn')) - if (this.resource.associatednetworkid) { - tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => ['vpn', 'firewall'].includes(tab.name))) - } - this.tabs = tabs - } else { - if (this.resource.associatednetworkid) { - this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'firewall')) - } else { - this.tabs = this.defaultTabs - } + this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => ['vpn', 'firewall'].includes(tab.name))) } return } // VPC IPs have all tabs, but firewall only if associatednetworkid present let tabs = this.$route.meta.tabs - if (!this.resource.associatednetworkid) { - tabs = tabs.filter(tab => tab.name !== 'firewall') - } const network = await this.fetchNetwork() if (network && network.networkofferingconservemode) { From 9e90c6236dd364da7f18ecfbbd59bfc54c3f8909 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Thu, 7 May 2026 15:04:18 +0530 Subject: [PATCH 04/32] UI fix --- ui/src/views/network/PublicIpResource.vue | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ui/src/views/network/PublicIpResource.vue b/ui/src/views/network/PublicIpResource.vue index f92270e576c9..cbe73d608244 100644 --- a/ui/src/views/network/PublicIpResource.vue +++ b/ui/src/views/network/PublicIpResource.vue @@ -151,12 +151,17 @@ export default { if (this.resource.isstaticnat) { if (this.resource.virtualmachinetype === 'DomainRouter') { this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => ['vpn', 'firewall'].includes(tab.name))) + } else { + this.tabs = this.defaultTabs } return } // VPC IPs have all tabs, but firewall only if associatednetworkid present let tabs = this.$route.meta.tabs + if (!this.resource.associatednetworkid) { + tabs = tabs.filter(tab => tab.name !== 'firewall') + } const network = await this.fetchNetwork() if (network && network.networkofferingconservemode) { From 4928aaf7c4b527a3a8bb6cc6e158836503a3367f Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Mon, 11 May 2026 12:01:53 +0530 Subject: [PATCH 05/32] Firewall tab changes --- systemvm/debian/opt/cloud/bin/configure.py | 14 ++++++++++ systemvm/debian/opt/cloud/bin/cs/CsAddress.py | 11 -------- ui/src/views/network/PublicIpResource.vue | 27 ++++++++++--------- 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/systemvm/debian/opt/cloud/bin/configure.py b/systemvm/debian/opt/cloud/bin/configure.py index b2c0676c65d0..a250229ba9e6 100755 --- a/systemvm/debian/opt/cloud/bin/configure.py +++ b/systemvm/debian/opt/cloud/bin/configure.py @@ -703,12 +703,26 @@ def process(self): self.add_routing_rules() return + fw_chains_created = set() for item in self.dbag: if item == "id": continue if self.config.is_vpc() and not ("purpose" in self.dbag[item] and self.dbag[item]["purpose"] == "Firewall"): self.AclDevice(self.dbag[item], self.config).create() else: + # For VPC firewall rules, create the PREROUTING jump and chain skeleton + # once per public IP before adding the individual rule + if self.config.is_vpc() and self.dbag[item].get("purpose") == "Firewall": + src_ip = self.dbag[item].get("src_ip") + if src_ip and src_ip not in fw_chains_created: + fw = self.config.get_fw() + fw.append(["mangle", "front", + "-A PREROUTING -d %s/32 -j FIREWALL_%s" % (src_ip, src_ip)]) + fw.append(["mangle", "front", + "-A FIREWALL_%s -m state --state RELATED,ESTABLISHED -j RETURN" % src_ip]) + fw.append(["mangle", "", + "-A FIREWALL_%s -j DROP" % src_ip]) + fw_chains_created.add(src_ip) self.AclIP(self.dbag[item], self.config).create() class CsIpv6Firewall(CsDataBag): diff --git a/systemvm/debian/opt/cloud/bin/cs/CsAddress.py b/systemvm/debian/opt/cloud/bin/cs/CsAddress.py index c3a4fe40ba1e..0ef5d45402da 100755 --- a/systemvm/debian/opt/cloud/bin/cs/CsAddress.py +++ b/systemvm/debian/opt/cloud/bin/cs/CsAddress.py @@ -647,17 +647,6 @@ def fw_vpcrouter(self): (self.address['network'], self.address['network'])]) if self.get_type() in ["public"]: - # Add PREROUTING firewall chain jump for public IP - self.fw.append(["mangle", "front", - "-A PREROUTING " + - "-d %s/32 -j FIREWALL_%s" % (self.address['public_ip'], self.address['public_ip'])]) - - # Add the firewall chain with default DROP policy - self.fw.append(["mangle", "front", - "-A FIREWALL_%s " % self.address['public_ip'] + - "-m state --state RELATED,ESTABLISHED -j RETURN"]) - self.fw.append(["mangle", "", - "-A FIREWALL_%s -j DROP" % self.address['public_ip']]) self.fw.append( ["mangle", "", "-A FORWARD -j VPN_STATS_%s" % self.dev]) diff --git a/ui/src/views/network/PublicIpResource.vue b/ui/src/views/network/PublicIpResource.vue index cbe73d608244..340aec7119f6 100644 --- a/ui/src/views/network/PublicIpResource.vue +++ b/ui/src/views/network/PublicIpResource.vue @@ -135,33 +135,27 @@ export default { return } if (this.resource && this.resource.vpcid) { - const vpc = await this.fetchVpc() +`` const vpc = await this.fetchVpc() // VPC IPs with source nat have only VPN when VPC offering conserve mode = false if (this.resource.issourcenat && vpc?.vpcofferingconservemode === false) { let tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'vpn')) - if (this.resource.associatednetworkid) { - tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => ['vpn', 'firewall'].includes(tab.name))) - } - this.tabs = tabs + this.tabs = this.addFirewallTab(tabs) return } - // VPC IPs with static nat have nothing + // VPC IPs with static nat keep existing VPN behavior and always show firewall if (this.resource.isstaticnat) { + let tabs = this.defaultTabs if (this.resource.virtualmachinetype === 'DomainRouter') { - this.tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => ['vpn', 'firewall'].includes(tab.name))) - } else { - this.tabs = this.defaultTabs + tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'vpn')) } + this.tabs = this.addFirewallTab(tabs) return } - // VPC IPs have all tabs, but firewall only if associatednetworkid present + // VPC IPs have all tabs, and firewall should always be visible let tabs = this.$route.meta.tabs - if (!this.resource.associatednetworkid) { - tabs = tabs.filter(tab => tab.name !== 'firewall') - } const network = await this.fetchNetwork() if (network && network.networkofferingconservemode) { @@ -209,6 +203,13 @@ export default { fetchAction () { this.actions = this.$route.meta.actions || [] }, + addFirewallTab (tabs) { + const firewallTab = this.$route.meta.tabs.find(tab => tab.name === 'firewall') + if (!firewallTab || tabs.some(tab => tab.name === 'firewall')) { + return tabs + } + return tabs.concat(firewallTab) + }, fetchVpc () { if (!this.resource.vpcid) { return null From 77535bf280284bd1e30131380b7130540299a8b9 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Mon, 11 May 2026 17:01:38 +0530 Subject: [PATCH 06/32] Changes in mapping to the VPC and not the tier network --- .../network/firewall/FirewallManagerImpl.java | 7 ++++++- ui/src/views/network/PublicIpResource.vue | 18 +++++++++++++----- 2 files changed, 19 insertions(+), 6 deletions(-) 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 744d7f5158d1..b0e08e5613ab 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -710,7 +710,12 @@ public boolean applyRules(Network network, Purpose purpose, List tab.name === 'vpn')) - this.tabs = this.addFirewallTab(tabs) + const tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'vpn')) + this.tabs = hasFirewallCapability ? this.addFirewallTab(tabs) : tabs return } @@ -150,12 +151,15 @@ export default { if (this.resource.virtualmachinetype === 'DomainRouter') { tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'vpn')) } - this.tabs = this.addFirewallTab(tabs) + this.tabs = hasFirewallCapability ? this.addFirewallTab(tabs) : tabs return } - // VPC IPs have all tabs, and firewall should always be visible + // VPC IPs have all tabs; firewall is shown only if VPC has firewall capability let tabs = this.$route.meta.tabs + if (!hasFirewallCapability) { + tabs = tabs.filter(tab => tab.name !== 'firewall') + } const network = await this.fetchNetwork() if (network && network.networkofferingconservemode) { @@ -210,6 +214,10 @@ export default { } return tabs.concat(firewallTab) }, + hasVpcFirewallCapability (vpc) { + const services = vpc?.service || [] + return Array.isArray(services) && services.some(service => (service?.name || '').toLowerCase() === 'firewall') + }, fetchVpc () { if (!this.resource.vpcid) { return null From d2a8e4b9cc2d4595fe65d26981fdc50ebb85ffe9 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 12 May 2026 13:47:57 +0530 Subject: [PATCH 07/32] Fix vpcid --- .../com/cloud/network/rules/FirewallRule.java | 4 +- .../firewall/CreateEgressFirewallRuleCmd.java | 2 +- .../user/firewall/CreateFirewallRuleCmd.java | 33 ++- .../firewall/CreatePortForwardingRuleCmd.java | 4 +- .../user/nat/CreateIpForwardingRuleCmd.java | 9 +- .../firewall/CreateFirewallRuleCmdTest.java | 29 ++ .../network/rules/StaticNatRuleImpl.java | 7 +- .../cloud/network/rules/FirewallRuleVO.java | 17 +- .../java/com/cloud/api/ApiResponseHelper.java | 9 +- .../cloud/network/IpAddressManagerImpl.java | 58 +++- .../com/cloud/network/NetworkModelImpl.java | 41 ++- .../element/VpcVirtualRouterElement.java | 8 +- .../network/firewall/FirewallManagerImpl.java | 273 ++++++++++++++++-- .../element/VpcVirtualRouterElementTest.java | 25 ++ .../network/firewall/FirewallManagerTest.java | 42 ++- 15 files changed, 503 insertions(+), 58 deletions(-) diff --git a/api/src/main/java/com/cloud/network/rules/FirewallRule.java b/api/src/main/java/com/cloud/network/rules/FirewallRule.java index 369c6aa57eb8..38ba009163ce 100644 --- a/api/src/main/java/com/cloud/network/rules/FirewallRule.java +++ b/api/src/main/java/com/cloud/network/rules/FirewallRule.java @@ -69,7 +69,9 @@ enum TrafficType { State getState(); - long getNetworkId(); + Long getNetworkId(); + + Long getVpcId(); Long getSourceIpAddressId(); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateEgressFirewallRuleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateEgressFirewallRuleCmd.java index 3fd571b7a479..e2c84ac85c7a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateEgressFirewallRuleCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateEgressFirewallRuleCmd.java @@ -212,7 +212,7 @@ public State getState() { } @Override - public long getNetworkId() { + public Long getNetworkId() { return networkId; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java index bc65126f33bd..30dd1a2d015a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java @@ -223,13 +223,9 @@ public State getState() { } @Override - public long getNetworkId() { - IpAddress ip = _entityMgr.findById(IpAddress.class, getIpAddressId()); - Long ntwkId = null; - - if (ip.getAssociatedWithNetworkId() != null) { - ntwkId = ip.getAssociatedWithNetworkId(); - } + public Long getNetworkId() { + IpAddress ip = getIp(); + Long ntwkId = isVpcIp(ip) ? getVpcNetworkIdForFirewallRule(ip) : getIsolatedNetworkIdForFirewallRule(ip); if (ntwkId == null) { throw new InvalidParameterValueException("Unable to create firewall rule for the IP address ID=" + ipAddressId + @@ -238,6 +234,12 @@ public long getNetworkId() { return ntwkId; } + @Override + public Long getVpcId() { + IpAddress ip = getIp(); + return isVpcIp(ip) ? ip.getVpcId() : null; + } + @Override public long getEntityOwnerId() { Account account = CallContext.current().getCallingAccount(); @@ -300,7 +302,21 @@ public String getSyncObjType() { @Override public Long getSyncObjId() { - return getIp().getAssociatedWithNetworkId(); + Long syncObjId = getIp().getAssociatedWithNetworkId(); + return syncObjId != null ? syncObjId : getNetworkId(); + } + + private boolean isVpcIp(IpAddress ip) { + return ip.getVpcId() != null; + } + + private Long getIsolatedNetworkIdForFirewallRule(IpAddress ip) { + return ip.getAssociatedWithNetworkId(); + } + + private Long getVpcNetworkIdForFirewallRule(IpAddress ip) { + // VPC flow is independent from tier association; manager resolves execution network. + return ip.getNetworkId(); } private IpAddress getIp() { @@ -311,6 +327,7 @@ private IpAddress getIp() { return ip; } + @Override public Integer getIcmpCode() { if (icmpCode != null) { 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 2bc5fc2ee68b..66fc118395ea 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 @@ -176,7 +176,7 @@ public Boolean getOpenFirewall() { } } - private Long getVpcId() { + public Long getVpcId() { if (ipAddressId != null) { IpAddress ipAddr = _networkService.getIp(ipAddressId); if (ipAddr == null || !ipAddr.readyToUse()) { @@ -275,7 +275,7 @@ public State getState() { } @Override - public long getNetworkId() { + public Long getNetworkId() { IpAddress ip = _entityMgr.findById(IpAddress.class, getIpAddressId()); Long ntwkId = _networkService.getPreferredNetworkIdForPublicIpRuleAssignment(ip, networkId); if (ntwkId == null) { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/nat/CreateIpForwardingRuleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/nat/CreateIpForwardingRuleCmd.java index 7963dfe5c7d3..98487ddeb19a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/nat/CreateIpForwardingRuleCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/nat/CreateIpForwardingRuleCmd.java @@ -229,8 +229,13 @@ public FirewallRule.State getState() { } @Override - public long getNetworkId() { - return -1; + public Long getNetworkId() { + return -1L; + } + + @Override + public Long getVpcId() { + return null; } @Override diff --git a/api/src/test/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmdTest.java index c905974b2be9..504d3914b70e 100644 --- a/api/src/test/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmdTest.java +++ b/api/src/test/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmdTest.java @@ -21,10 +21,15 @@ import java.util.Collections; import java.util.List; +import com.cloud.network.IpAddress; +import com.cloud.network.NetworkService; +import com.cloud.utils.db.EntityManager; import org.apache.commons.collections.CollectionUtils; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import org.springframework.test.util.ReflectionTestUtils; @@ -33,6 +38,12 @@ @RunWith(MockitoJUnitRunner.class) public class CreateFirewallRuleCmdTest { + @Mock + private EntityManager entityManager; + + @Mock + private NetworkService networkService; + private void validateAllIp4Cidr(final CreateFirewallRuleCmd cmd) { Assert.assertTrue(CollectionUtils.isNotEmpty(cmd.getSourceCidrList())); Assert.assertEquals(1, cmd.getSourceCidrList().size()); @@ -88,4 +99,22 @@ public void testGetSourceCidrList_EmptyFirstElementButMore() { Assert.assertEquals(2, cmd.getSourceCidrList().size()); Assert.assertEquals(cidr, cmd.getSourceCidrList().get(1)); } + + @Test + public void testGetNetworkIdVpcWithoutAssociatedNetworkUsesVpcFallbackAndSyncObjId() { + final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd(); + final IpAddress ip = Mockito.mock(IpAddress.class); + + cmd._entityMgr = entityManager; + cmd._networkService = networkService; + ReflectionTestUtils.setField(cmd, "ipAddressId", 42L); + + Mockito.when(networkService.getIp(42L)).thenReturn(ip); + Mockito.when(ip.getAssociatedWithNetworkId()).thenReturn(null); + Mockito.when(ip.getVpcId()).thenReturn(100L); + Mockito.when(ip.getNetworkId()).thenReturn(2L); + + Assert.assertEquals(Long.valueOf(2L), cmd.getNetworkId()); + Assert.assertEquals(Long.valueOf(2L), cmd.getSyncObjId()); + } } diff --git a/engine/components-api/src/main/java/com/cloud/network/rules/StaticNatRuleImpl.java b/engine/components-api/src/main/java/com/cloud/network/rules/StaticNatRuleImpl.java index 4d8270ca078d..980606176024 100644 --- a/engine/components-api/src/main/java/com/cloud/network/rules/StaticNatRuleImpl.java +++ b/engine/components-api/src/main/java/com/cloud/network/rules/StaticNatRuleImpl.java @@ -80,10 +80,15 @@ public long getDomainId() { } @Override - public long getNetworkId() { + public Long getNetworkId() { return networkId; } + @Override + public Long getVpcId() { + return null; + } + @Override public long getId() { return id; diff --git a/engine/schema/src/main/java/com/cloud/network/rules/FirewallRuleVO.java b/engine/schema/src/main/java/com/cloud/network/rules/FirewallRuleVO.java index 6ce9e6a118b7..2b34e23fa4d4 100644 --- a/engine/schema/src/main/java/com/cloud/network/rules/FirewallRuleVO.java +++ b/engine/schema/src/main/java/com/cloud/network/rules/FirewallRuleVO.java @@ -91,6 +91,9 @@ public class FirewallRuleVO implements FirewallRule { @Column(name = "network_id") Long networkId; + @Column(name = "vpc_id") + Long vpcId; + @Column(name = "icmp_code") Integer icmpCode; @@ -196,10 +199,18 @@ public State getState() { } @Override - public long getNetworkId() { + public Long getNetworkId() { return networkId; } + public Long getVpcId() { + return vpcId; + } + + public void setVpcId(Long vpcId) { + this.vpcId = vpcId; + } + @Override public FirewallRuleType getType() { return type; @@ -217,7 +228,7 @@ protected FirewallRuleVO() { uuid = UUID.randomUUID().toString(); } - public FirewallRuleVO(String xId, Long ipAddressId, Integer portStart, Integer portEnd, String protocol, long networkId, long accountId, long domainId, + public FirewallRuleVO(String xId, Long ipAddressId, Integer portStart, Integer portEnd, String protocol, Long networkId, long accountId, long domainId, Purpose purpose, List sourceCidrs, Integer icmpCode, Integer icmpType, Long related, TrafficType trafficType) { this.xId = xId; if (xId == null) { @@ -261,7 +272,7 @@ public FirewallRuleVO(String xId, long ipAddressId, int port, String protocol, l } - public FirewallRuleVO(String xId, Long ipAddressId, Integer portStart, Integer portEnd, String protocol, long networkId, long accountId, long domainId, + public FirewallRuleVO(String xId, Long ipAddressId, Integer portStart, Integer portEnd, String protocol, Long networkId, long accountId, long domainId, Purpose purpose, List sourceCidrs, List destCidrs, Integer icmpCode, Integer icmpType, Long related, TrafficType trafficType) { this(xId,ipAddressId, portStart, portEnd, protocol, networkId, accountId, domainId, purpose, sourceCidrs, icmpCode, icmpType, related, trafficType); this.destinationCidrs = destCidrs; diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index 49b0e342efc6..3a0d4cecc78d 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -2968,8 +2968,13 @@ public FirewallResponse createFirewallResponse(FirewallRule fwRule) { } } - Network network = ApiDBUtils.findNetworkById(fwRule.getNetworkId()); - response.setNetworkId(network.getUuid()); + Long networkId = fwRule.getNetworkId(); + if (networkId != null) { + Network network = ApiDBUtils.findNetworkById(networkId); + if (network != null) { + response.setNetworkId(network.getUuid()); + } + } FirewallRule.State state = fwRule.getState(); String stateToSet = state.toString(); diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index da84e5058603..b7e147414e74 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -659,28 +659,60 @@ public boolean applyRules(List rules, FirewallRule.Purpo } boolean success = true; - Network network = _networksDao.findById(rules.get(0).getNetworkId()); - FirewallRuleVO.TrafficType trafficType = rules.get(0).getTrafficType(); + FirewallRule firstRule = rules.get(0); + Long networkId = firstRule.getNetworkId(); + Long vpcId = firstRule.getVpcId(); + FirewallRuleVO.TrafficType trafficType = firstRule.getTrafficType(); List publicIps = new ArrayList(); - if (!(rules.get(0).getPurpose() == FirewallRule.Purpose.Firewall && trafficType == FirewallRule.TrafficType.Egress)) { - // get the list of public ip's owned by the network - List userIps = _ipAddressDao.listByAssociatedNetwork(network.getId(), null); - if (userIps != null && !userIps.isEmpty()) { - for (IPAddressVO userIp : userIps) { - PublicIp publicIp = PublicIp.createFromAddrAndVlan(userIp, _vlanDao.findById(userIp.getVlanId())); - publicIps.add(publicIp); + // For VPC firewall rules the networkId on the rule is null; resolve via VPC. + Network network; + if (networkId != null) { + network = _networksDao.findById(networkId); + } else if (vpcId != null) { + List vpcNetworks = _vpcMgr.getVpcNetworks(vpcId); + network = (vpcNetworks != null && !vpcNetworks.isEmpty()) ? _networksDao.findById(vpcNetworks.get(0).getId()) : null; + } else { + network = null; + } + + if (network == null) { + logger.warn("Unable to resolve network for firewall rules (networkId={}, vpcId={}); skipping IP association", networkId, vpcId); + } else if (!(firstRule.getPurpose() == FirewallRule.Purpose.Firewall && trafficType == FirewallRule.TrafficType.Egress)) { + // For VPC ingress rules, collect public IPs tied to the VPC rather than network association + if (vpcId != null && networkId == null) { + List vpcIps = _ipAddressDao.listByAssociatedVpc(vpcId, null); + if (vpcIps != null) { + for (IPAddressVO userIp : vpcIps) { + PublicIp publicIp = PublicIp.createFromAddrAndVlan(userIp, _vlanDao.findById(userIp.getVlanId())); + publicIps.add(publicIp); + } + } + } else { + // get the list of public ip's owned by the network + List userIps = _ipAddressDao.listByAssociatedNetwork(network.getId(), null); + if (userIps != null && !userIps.isEmpty()) { + for (IPAddressVO userIp : userIps) { + PublicIp publicIp = PublicIp.createFromAddrAndVlan(userIp, _vlanDao.findById(userIp.getVlanId())); + publicIps.add(publicIp); + } } } } - // rules can not programmed unless IP is associated with network service provider, so run IP assoication for + + // rules can not programmed unless IP is associated with network service provider, so run IP association for // the network so as to ensure IP is associated before applying rules (in add state) - if (checkIfIpAssocRequired(network, false, publicIps)) { + if (network != null && checkIfIpAssocRequired(network, false, publicIps)) { applyIpAssociations(network, false, continueOnError, publicIps); } try { - applier.applyRules(network, purpose, rules); + if (network != null) { + applier.applyRules(network, purpose, rules); + } else { + logger.warn("Skipping applyRules: no network resolved for rules (vpcId={})", vpcId); + success = false; + } } catch (ResourceUnavailableException e) { if (!continueOnError) { throw e; @@ -691,7 +723,7 @@ public boolean applyRules(List rules, FirewallRule.Purpo // if there are no active rules associated with a public IP, then public IP need not be associated with a provider. // This IPAssoc ensures, public IP is dis-associated after last active rule is revoked. - if (checkIfIpAssocRequired(network, true, publicIps)) { + if (network != null && checkIfIpAssocRequired(network, true, publicIps)) { applyIpAssociations(network, true, continueOnError, publicIps); } diff --git a/server/src/main/java/com/cloud/network/NetworkModelImpl.java b/server/src/main/java/com/cloud/network/NetworkModelImpl.java index a9ca5004a06c..f47046cdc434 100644 --- a/server/src/main/java/com/cloud/network/NetworkModelImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkModelImpl.java @@ -107,9 +107,11 @@ import com.cloud.network.rules.dao.PortForwardingRulesDao; import com.cloud.network.vpc.Vpc; import com.cloud.network.vpc.VpcGatewayVO; +import com.cloud.network.vpc.VpcOfferingServiceMapVO; import com.cloud.network.vpc.dao.PrivateIpDao; import com.cloud.network.vpc.dao.VpcDao; import com.cloud.network.vpc.dao.VpcGatewayDao; +import com.cloud.network.vpc.dao.VpcOfferingServiceMapDao; import com.cloud.offering.NetworkOffering; import com.cloud.offering.NetworkOffering.Detail; import com.cloud.offerings.NetworkOfferingServiceMapVO; @@ -186,6 +188,8 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi NetworkPermissionDao _networkPermissionDao; @Inject VpcDao vpcDao; + @Inject + VpcOfferingServiceMapDao _vpcOffSvcMapDao; private List networkElements; @@ -510,12 +514,16 @@ public boolean canIpUsedForService(PublicIp publicIp, Service service, Long netw // We only support one provider for one service now Map> serviceToProviders = getServiceProvidersMap(networkId); // Since IP already has service to bind with, the oldProvider can't be null - Set newProviders = serviceToProviders.get(service); + Set newProviders = getProvidersForServiceWithVpcFallback(serviceToProviders, service, publicIp.getVpcId()); if (newProviders == null || newProviders.isEmpty()) { throw new InvalidParameterValueException("There is no new provider for IP " + publicIp.getAddress() + " of service " + service.getName() + "!"); } Provider newProvider = (Provider)newProviders.toArray()[0]; - Set oldProviders = serviceToProviders.get(services.toArray()[0]); + Service existingService = (Service) services.toArray()[0]; + Set oldProviders = getProvidersForServiceWithVpcFallback(serviceToProviders, existingService, publicIp.getVpcId()); + if (oldProviders == null || oldProviders.isEmpty()) { + throw new InvalidParameterValueException("There is no existing provider for IP " + publicIp.getAddress() + " of service " + existingService.getName() + "!"); + } Provider oldProvider = (Provider)oldProviders.toArray()[0]; Network network = _networksDao.findById(networkId); NetworkElement oldElement = getElementImplementingProvider(oldProvider.getName()); @@ -530,6 +538,35 @@ public boolean canIpUsedForService(PublicIp publicIp, Service service, Long netw return true; } + private Set getProvidersForServiceWithVpcFallback(Map> serviceToProviders, Service service, Long vpcId) { + Set providers = serviceToProviders.get(service); + if (providers != null && !providers.isEmpty()) { + return providers; + } + + if (vpcId == null || service != Service.Firewall) { + return providers; + } + + Set vpcProviders = new HashSet(); + Vpc vpc = vpcDao.findById(vpcId); + if (vpc == null) { + return vpcProviders; + } + + List offeringProviders = _vpcOffSvcMapDao.listProvidersForServiceForVpcOffering(vpc.getVpcOfferingId(), Service.Firewall); + if (offeringProviders != null) { + for (VpcOfferingServiceMapVO offeringProvider : offeringProviders) { + Provider provider = Provider.getProvider(offeringProvider.getProvider()); + if (provider != null) { + vpcProviders.add(provider); + } + } + } + + return vpcProviders; + } + Map> getProviderServicesMap(long networkId) { Map> map = new HashMap>(); List nsms = _ntwkSrvcDao.getServicesInNetwork(networkId); diff --git a/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java b/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java index 2b12c1a9f13d..c1311c96eb73 100644 --- a/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java +++ b/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java @@ -139,7 +139,13 @@ protected boolean canHandle(final Network network, final Service service) { return false; } } else { - if (!_networkMdl.isProviderSupportServiceInNetwork(network.getId(), service, getProvider())) { + boolean supportsService; + if (service == Service.Firewall) { + supportsService = _vpcMgr.isProviderSupportServiceInVpc(network.getVpcId(), service, getProvider()); + } else { + supportsService = _networkMdl.isProviderSupportServiceInNetwork(network.getId(), service, getProvider()); + } + if (!supportsService) { logger.trace("Element " + getProvider().getName() + " doesn't support service " + service.getName() + " in the network " + network); return false; } 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 b0e08e5613ab..ca2183f75858 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -203,25 +203,62 @@ public FirewallRule createEgressFirewallRule(FirewallRule rule) throws NetworkRu if (sourceCidrs != null && !sourceCidrs.isEmpty()) Collections.replaceAll(sourceCidrs, "0.0.0.0/0", network.getCidr()); - return createFirewallRule(null, caller, rule.getXid(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), sourceCidrs, rule.getDestinationCidrList(), - rule.getIcmpCode(), rule.getIcmpType(), null, rule.getType(), rule.getNetworkId(), rule.getTrafficType(), rule.isDisplay()); + return createFirewallRuleForNonVPC(null, caller, rule.getXid(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), sourceCidrs, + rule.getDestinationCidrList(), rule.getIcmpCode(), rule.getIcmpType(), null, rule.getType(), rule.getNetworkId(), rule.getTrafficType(), rule.isDisplay()); } @Override @ActionEvent(eventType = EventTypes.EVENT_FIREWALL_OPEN, eventDescription = "creating firewall rule", create = true) public FirewallRule createIngressFirewallRule(FirewallRule rule) throws NetworkRuleConflictException { - Account caller = CallContext.current().getCallingAccount(); + Account caller = CallContext.current().getCallingAccount(); Long sourceIpAddressId = rule.getSourceIpAddressId(); + IPAddressVO sourceIp = getSourceIpForIngressRule(sourceIpAddressId); + + if (sourceIp.getVpcId() != null) { + return createIngressFirewallRuleForVpcIp(rule, caller, sourceIp); + } + return createIngressFirewallRuleForIsolatedIp(rule, caller, sourceIp); + } + + protected IPAddressVO getSourceIpForIngressRule(Long sourceIpAddressId) { + if (sourceIpAddressId == null) { + return null; + } + return _ipAddressDao.findById(sourceIpAddressId); + } + + protected FirewallRule createIngressFirewallRuleForIsolatedIp(FirewallRule rule, Account caller, IPAddressVO sourceIp) + throws NetworkRuleConflictException { + return createFirewallRuleForNonVPC(rule.getSourceIpAddressId(), 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()); + } - 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()); + protected FirewallRule createIngressFirewallRuleForVpcIp(FirewallRule rule, Account caller, IPAddressVO sourceIp) + throws NetworkRuleConflictException { + Long vpcId = sourceIp != null ? sourceIp.getVpcId() : null; + return createFirewallRuleForVpc(rule.getSourceIpAddressId(), caller, rule.getXid(), rule.getSourcePortStart(), rule.getSourcePortEnd(), + rule.getProtocol(), rule.getSourceCidrList(), null, rule.getIcmpCode(), rule.getIcmpType(), null, rule.getType(), + vpcId, 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 sourceCidrList, final List 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 { + final FirewallRule.FirewallRuleType type, final Long networkId, final Long vpcId, final FirewallRule.TrafficType trafficType, final Boolean forDisplay) throws NetworkRuleConflictException { + if (vpcId != null) { + return createFirewallRuleForVpc(ipAddrId, caller, xId, portStart, portEnd, protocol, sourceCidrList, destCidrList, icmpCode, icmpType, relatedRuleId, + type, vpcId, trafficType, forDisplay); + } + return createFirewallRuleForNonVPC(ipAddrId, caller, xId, portStart, portEnd, protocol, sourceCidrList, destCidrList, icmpCode, icmpType, relatedRuleId, + type, networkId, trafficType, forDisplay); + } + + @DB + protected FirewallRule createFirewallRuleForNonVPC(final Long ipAddrId, Account caller, final String xId, final Integer portStart, final Integer portEnd, final String protocol, + final List sourceCidrList, final List 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; try { // Validate ip address @@ -288,6 +325,158 @@ protected FirewallRule createFirewallRule(final Long ipAddrId, Account caller, f } } + @DB + protected FirewallRule createFirewallRuleForVpc(final Long ipAddrId, Account caller, final String xId, final Integer portStart, final Integer portEnd, final String protocol, + final List sourceCidrList, final List destCidrList, final Integer icmpCode, final Integer icmpType, + final Long relatedRuleId, final FirewallRuleType type, final Long vpcId, + final FirewallRule.TrafficType trafficType, final Boolean forDisplay) throws NetworkRuleConflictException { + IPAddressVO ipAddress = null; + try { + Long resolvedVpcId = vpcId; + if (ipAddrId != 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"); + } + resolvedVpcId = resolvedVpcId != null ? resolvedVpcId : ipAddress.getVpcId(); + } + + if (resolvedVpcId == null) { + throw new InvalidParameterValueException("Unable to create VPC firewall rule; couldn't locate VPC id"); + } + + validateFirewallRuleForVpc(caller, ipAddress, portStart, portEnd, protocol, Purpose.Firewall, type, resolvedVpcId, trafficType); + + 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"); + } + + Long accountId = null; + Long domainId = null; + + if (ipAddress != null) { + accountId = ipAddress.getAllocatedToAccountId(); + domainId = ipAddress.getAllocatedInDomainId(); + } else { + Vpc vpc = _vpcMgr.getActiveVpc(resolvedVpcId); + if (vpc == null) { + throw new InvalidParameterValueException("Unable to create VPC firewall rule; couldn't locate VPC by id=" + resolvedVpcId); + } + accountId = vpc.getAccountId(); + domainId = vpc.getDomainId(); + } + + final Long accountIdFinal = accountId; + final Long domainIdFinal = domainId; + final Long resolvedNetworkIdFinal = null; + final Long resolvedVpcIdFinal = resolvedVpcId; + return Transaction.execute((TransactionCallbackWithException) status -> { + FirewallRuleVO newRule = new FirewallRuleVO(xId, ipAddrId, portStart, portEnd, protocol.toLowerCase(), resolvedNetworkIdFinal, accountIdFinal, domainIdFinal, Purpose.Firewall, + sourceCidrList, destCidrList, icmpCode, icmpType, relatedRuleId, trafficType); + newRule.setVpcId(resolvedVpcIdFinal); + newRule.setType(type); + if (forDisplay != null) { + newRule.setDisplay(forDisplay); + } + newRule = _firewallDao.persist(newRule); + + if (type == FirewallRuleType.User) + detectRulesConflict(newRule); + + if (!_firewallDao.setStateToAdd(newRule)) { + throw new CloudRuntimeException("Unable to update the state to add for " + newRule); + } + CallContext.current().setEventDetails("Rule ID: " + newRule.getUuid()); + CallContext.current().putContextParameter(FirewallRule.class, newRule.getId()); + + return newRule; + }); + } finally { + if (ipAddrId != null) { + _ipAddressDao.releaseFromLockTable(ipAddrId); + } + } + } + + protected void validateFirewallRuleForVpc(Account caller, IPAddressVO ipAddress, Integer portStart, Integer portEnd, String proto, Purpose purpose, + FirewallRuleType type, Long vpcId, FirewallRule.TrafficType trafficType) { + if (portStart != null && !NetUtils.isValidPort(portStart)) { + throw new InvalidParameterValueException("publicPort is an invalid value: " + portStart); + } + if (portEnd != null && !NetUtils.isValidPort(portEnd)) { + throw new InvalidParameterValueException("Public port range is an invalid value: " + portEnd); + } + if (portStart != null && portEnd != null && portStart > portEnd) { + throw new InvalidParameterValueException("Start port can't be bigger than end port"); + } + + if (ipAddress == null && type == FirewallRuleType.System) { + return; + } + + if (vpcId == null) { + throw new InvalidParameterValueException("Unable to retrieve VPC id to validate the rule"); + } + + if (ipAddress != null) { + _accountMgr.checkAccess(caller, null, true, ipAddress); + } + + Vpc vpc = _vpcMgr.getActiveVpc(vpcId); + if (vpc == null) { + throw new InvalidParameterValueException("Unable to retrieve VPC to validate the rule by id=" + vpcId); + } + + Map caps = null; + if (purpose == Purpose.Firewall) { + caps = getFirewallServiceCapabilitiesForVpc(vpcId); + } + + if (caps != null) { + String supportedTrafficTypes = null; + if (purpose == FirewallRule.Purpose.Firewall) { + supportedTrafficTypes = caps.get(Capability.SupportedTrafficDirection).toLowerCase(); + } + + String supportedProtocols; + if (purpose == FirewallRule.Purpose.Firewall && trafficType == FirewallRule.TrafficType.Egress) { + supportedProtocols = caps.get(Capability.SupportedEgressProtocols).toLowerCase(); + } else { + supportedProtocols = caps.get(Capability.SupportedProtocols).toLowerCase(); + } + + if (!supportedProtocols.contains(proto.toLowerCase())) { + throw new InvalidParameterValueException("Protocol " + proto + " is not supported in VPC " + vpcId); + } else if (proto.equalsIgnoreCase(NetUtils.ICMP_PROTO) && purpose != Purpose.Firewall) { + throw new InvalidParameterValueException("Protocol " + proto + " is currently supported only for rules with purpose " + Purpose.Firewall); + } else if (purpose == Purpose.Firewall && !supportedTrafficTypes.contains(trafficType.toString().toLowerCase())) { + throw new InvalidParameterValueException(String.format("Traffic Type %s is currently supported by Firewall in VPC %s", trafficType, vpc.getUuid())); + } + } + } + + protected Map getFirewallServiceCapabilitiesForVpc(Long vpcId) { + for (FirewallServiceProvider fwElement : _firewallElements) { + Network.Provider provider = fwElement.getProvider(); + if (_vpcMgr.isProviderSupportServiceInVpc(vpcId, Service.Firewall, provider)) { + Map> capabilities = fwElement.getCapabilities(); + if (capabilities != null && capabilities.get(Service.Firewall) != null) { + return capabilities.get(Service.Firewall); + } + } + } + return null; + } + + protected Long resolveIsolatedFirewallRuleNetworkId(IPAddressVO ipAddress, Long networkId) { + _networkModel.checkIpForService(ipAddress, Service.Firewall, networkId); + return ipAddress.getAssociatedWithNetworkId(); + } + @Override public Pair, Integer> listFirewallRules(IListFirewallRulesCmd cmd) { Long ipId = cmd.getIpAddressId(); @@ -404,9 +593,16 @@ public void detectRulesConflict(FirewallRule newRule) throws NetworkRuleConflict assert (rules.size() >= 1); } - NetworkVO newRuleNetwork = getNewRuleNetwork(newRule); - boolean newRuleIsOnVpcNetwork = newRuleNetwork.getVpcId() != null; - boolean vpcConserveModeEnabled = _vpcMgr.isNetworkOnVpcEnabledConserveMode(newRuleNetwork); + Long newRuleVpcId = newRule.getVpcId(); + boolean newRuleIsVpc = newRuleVpcId != null; + NetworkVO newRuleNetwork = null; + boolean newRuleIsOnVpcNetwork = false; + boolean vpcConserveModeEnabled = false; + if (!newRuleIsVpc) { + newRuleNetwork = getNewRuleNetwork(newRule); + newRuleIsOnVpcNetwork = newRuleNetwork.getVpcId() != null; + vpcConserveModeEnabled = newRuleIsOnVpcNetwork && _vpcMgr.isNetworkOnVpcEnabledConserveMode(newRuleNetwork); + } for (FirewallRuleVO rule : rules) { if (rule.getId() == newRule.getId()) { @@ -457,8 +653,8 @@ public void detectRulesConflict(FirewallRule newRule) throws NetworkRuleConflict // Checking if the rule applied is to the same network that is passed in the rule. // (except for VPCs with conserve mode = true) - if ((!newRuleIsOnVpcNetwork || !vpcConserveModeEnabled) - && rule.getNetworkId() != newRule.getNetworkId() && rule.getState() != State.Revoke) { + if (!newRuleIsVpc && (!newRuleIsOnVpcNetwork || !vpcConserveModeEnabled) + && !Objects.equals(rule.getNetworkId(), newRule.getNetworkId()) && rule.getState() != State.Revoke) { String errMsg = String.format("New rule is for a different network than what's specified in rule %s", rule.getXid()); if (newRuleIsOnVpcNetwork) { Vpc vpc = _vpcMgr.getActiveVpc(newRuleNetwork.getVpcId()); @@ -580,11 +776,9 @@ public void validateFirewallRule(Account caller, IPAddressVO ipAddress, Integer } if (ipAddress != null) { - if (ipAddress.getAssociatedWithNetworkId() == null) { - throw new InvalidParameterValueException("Unable to create firewall rule ; ip with specified id is not associated with any network"); - } else { - networkId = ipAddress.getAssociatedWithNetworkId(); - } + networkId = isVpcIpAddress(ipAddress) + ? validateFirewallRuleForVpcIp(ipAddress, networkId) + : validateFirewallRuleForIsolatedIp(ipAddress); // Validate ip address _accountMgr.checkAccess(caller, null, true, ipAddress); @@ -615,7 +809,7 @@ public void validateFirewallRule(Account caller, IPAddressVO ipAddress, Integer if (routedIpv4Manager.isVirtualRouterGateway(network)) { throw new CloudRuntimeException("Unable to create routing firewall rule. Please use routing firewall API instead."); } - caps = _networkModel.getNetworkServiceCapabilities(network.getId(), Service.Firewall); + caps = getFirewallServiceCapabilities(network); } if (caps != null) { @@ -655,6 +849,41 @@ public void validateFirewallRule(Account caller, IPAddressVO ipAddress, Integer } + protected boolean isVpcIpAddress(IPAddressVO ipAddress) { + return ipAddress.getVpcId() != null; + } + + protected Long validateFirewallRuleForIsolatedIp(IPAddressVO ipAddress) { + if (ipAddress.getAssociatedWithNetworkId() == null) { + throw new InvalidParameterValueException("Unable to create firewall rule ; ip with specified id is not associated with any network"); + } + return ipAddress.getAssociatedWithNetworkId(); + } + + protected Long validateFirewallRuleForVpcIp(IPAddressVO ipAddress, Long networkId) { + if (networkId == null) { + throw new InvalidParameterValueException("Unable to retrieve network id to validate the rule"); + } + return networkId; + } + + protected Map getFirewallServiceCapabilities(Network network) { + if (network.getVpcId() == null) { + return _networkModel.getNetworkServiceCapabilities(network.getId(), Service.Firewall); + } + + for (FirewallServiceProvider fwElement : _firewallElements) { + Network.Provider provider = fwElement.getProvider(); + if (_vpcMgr.isProviderSupportServiceInVpc(network.getVpcId(), Service.Firewall, provider)) { + Map> capabilities = fwElement.getCapabilities(); + if (capabilities != null && capabilities.get(Service.Firewall) != null) { + return capabilities.get(Service.Firewall); + } + } + } + return _networkModel.getNetworkServiceCapabilities(network.getId(), Service.Firewall); + } + @Override public boolean applyRules(List rules, boolean continueOnError, boolean updateRulesInDB) throws ResourceUnavailableException { boolean success = true; @@ -822,8 +1051,10 @@ public boolean applyFirewallRules(List rules, boolean continueOn for (FirewallRuleVO rule : rules) { // validate rule - for NSX - long networkId = rule.getNetworkId(); - validateNsxConstraints(networkId, rule); + Long networkId = rule.getNetworkId(); + if (networkId != null) { + validateNsxConstraints(networkId, rule); + } // load cidrs if any rule.setSourceCidrList(_firewallCidrsDao.getSourceCidrs(rule.getId())); rule.setDestinationCidrsList(_firewallDcidrsDao.getDestCidrs(rule.getId())); @@ -1083,7 +1314,7 @@ public FirewallRule createRuleForAllCidrs(long ipAddrId, Account caller, Integer List oneCidr = new ArrayList(); oneCidr.add(NetUtils.ALL_IP4_CIDRS); return createFirewallRule(ipAddrId, caller, null, startPort, endPort, protocol, oneCidr, null, icmpCode, icmpType, relatedRuleId, FirewallRule.FirewallRuleType.User, - networkId, FirewallRule.TrafficType.Ingress, true); + networkId, null, FirewallRule.TrafficType.Ingress, true); } @Override @@ -1198,7 +1429,7 @@ public boolean addSystemFirewallRules(IPAddressVO ip, Account acct) { _firewallDao.loadSourceCidrs(rule); } createFirewallRule(ip.getId(), acct, rule.getXid(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), rule.getSourceCidrList(),null, - rule.getIcmpCode(), rule.getIcmpType(), rule.getRelated(), FirewallRuleType.System, rule.getNetworkId(), rule.getTrafficType(), true); + rule.getIcmpCode(), rule.getIcmpType(), rule.getRelated(), FirewallRuleType.System, rule.getNetworkId(), rule.getVpcId(), rule.getTrafficType(), true); } catch (Exception e) { logger.debug("Failed to add system wide firewall rule, due to:" + e.toString()); } diff --git a/server/src/test/java/com/cloud/network/element/VpcVirtualRouterElementTest.java b/server/src/test/java/com/cloud/network/element/VpcVirtualRouterElementTest.java index 20ddb39d9432..ede1feb74288 100644 --- a/server/src/test/java/com/cloud/network/element/VpcVirtualRouterElementTest.java +++ b/server/src/test/java/com/cloud/network/element/VpcVirtualRouterElementTest.java @@ -19,10 +19,13 @@ import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.DataCenterDao; import com.cloud.exception.ResourceUnavailableException; +import com.cloud.network.Network; +import com.cloud.network.NetworkModel; import com.cloud.network.RemoteAccessVpn; import com.cloud.network.VpnUser; import com.cloud.network.router.VpcVirtualNetworkApplianceManagerImpl; import com.cloud.network.vpc.Vpc; +import com.cloud.network.vpc.VpcManager; import com.cloud.network.vpc.dao.VpcDao; import com.cloud.utils.db.EntityManager; import com.cloud.vm.DomainRouterVO; @@ -43,7 +46,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -60,6 +65,12 @@ public class VpcVirtualRouterElementTest { @Mock EntityManager _entityMgr; + @Mock + NetworkModel _networkMdl; + + @Mock + VpcManager _vpcMgr; + @Mock NetworkTopologyContext networkTopologyContext; @@ -188,4 +199,18 @@ public void testApplyVpnUsersException2() { verify(remoteAccessVpn, times(1)).getVpcId(); } + + @Test + public void testCanHandleFirewallUsesVpcCapability() { + final Network network = Mockito.mock(Network.class); + + when(_networkMdl.getPhysicalNetworkId(network)).thenReturn(1L); + when(network.getVpcId()).thenReturn(100L); + when(_networkMdl.isProviderEnabledInPhysicalNetwork(1L, Network.Provider.VPCVirtualRouter.getName())).thenReturn(true); + when(_vpcMgr.isProviderSupportServiceInVpc(100L, Network.Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(true); + + assertTrue(vpcVirtualRouterElement.canHandle(network, Network.Service.Firewall)); + verify(_vpcMgr).isProviderSupportServiceInVpc(100L, Network.Service.Firewall, Network.Provider.VPCVirtualRouter); + verify(_networkMdl, never()).isProviderSupportServiceInNetwork(network.getId(), Network.Service.Firewall, Network.Provider.VPCVirtualRouter); + } } 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 bacef85479a2..e570880545e9 100644 --- a/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java +++ b/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java @@ -21,23 +21,29 @@ import com.cloud.exception.ResourceUnavailableException; import com.cloud.network.IpAddressManager; import com.cloud.network.Network; +import com.cloud.network.Network.Capability; +import com.cloud.network.Network.Service; import com.cloud.network.NetworkModel; import com.cloud.network.NetworkRuleApplier; import com.cloud.network.dao.FirewallRulesDao; +import com.cloud.network.dao.IPAddressVO; 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; import com.cloud.network.rules.FirewallRule; +import com.cloud.network.rules.FirewallRule.FirewallRuleType; import com.cloud.network.rules.FirewallRule.Purpose; import com.cloud.network.rules.FirewallRuleVO; import com.cloud.network.vpc.Vpc; import com.cloud.network.vpc.VpcManager; +import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.DomainManager; import com.cloud.utils.component.ComponentContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; +import org.apache.cloudstack.network.RoutedIpv4Manager; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -53,7 +59,9 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; @@ -79,6 +87,8 @@ public class FirewallManagerTest { @Mock IpAddressManager _ipAddrMgr; @Mock + RoutedIpv4Manager routedIpv4Manager; + @Mock FirewallRulesDao _firewallDao; @Mock NetworkDao _networkDao; @@ -115,7 +125,7 @@ public void tearDown() throws Exception { } private FirewallRule createFirewallRule(int startPort, int endPort, Purpose purpose) { - return new FirewallRuleVO("xid", 1L, startPort, endPort, "TCP", 2, 3, 4, purpose, new ArrayList<>(), + return new FirewallRuleVO("xid", 1L, startPort, endPort, "TCP", 2L, 3, 4, purpose, new ArrayList<>(), new ArrayList<>(), 5, 6, null, FirewallRule.TrafficType.Ingress); } @@ -332,4 +342,34 @@ public void checkIfRulesHaveConflictingPortRangesTestBothRulesArePortForwardingA Assert.assertFalse(result); } + + @Test + public void testValidateFirewallRuleVpcWithoutAssociatedNetworkUsesVpcCapabilities() { + final Account caller = Mockito.mock(Account.class); + final IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); + final NetworkVO network = Mockito.mock(NetworkVO.class); + final FirewallServiceProvider firewallServiceProvider = Mockito.mock(FirewallServiceProvider.class); + final Map firewallCaps = new HashMap<>(); + final Map> capabilities = new HashMap<>(); + + firewallCaps.put(Capability.SupportedTrafficDirection, "ingress, egress"); + firewallCaps.put(Capability.SupportedProtocols, "tcp,udp,icmp"); + firewallCaps.put(Capability.SupportedEgressProtocols, "tcp,udp,icmp"); + capabilities.put(Service.Firewall, firewallCaps); + + when(ipAddress.getAssociatedWithNetworkId()).thenReturn(null); + when(ipAddress.getVpcId()).thenReturn(10L); + when(_networkModel.getNetwork(2L)).thenReturn(network); + when(network.getVpcId()).thenReturn(10L); + when(network.getDataCenterId()).thenReturn(1L); + when(routedIpv4Manager.isVirtualRouterGateway(network)).thenReturn(false); + when(firewallServiceProvider.getProvider()).thenReturn(Network.Provider.VPCVirtualRouter); + when(firewallServiceProvider.getCapabilities()).thenReturn(capabilities); + when(_vpcMgr.isProviderSupportServiceInVpc(10L, Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(true); + _firewallMgr._firewallElements = List.of(firewallServiceProvider); + + _firewallMgr.validateFirewallRule(caller, ipAddress, 80, 80, "tcp", Purpose.Firewall, FirewallRuleType.User, 2L, FirewallRule.TrafficType.Ingress); + + verify(_networkModel, Mockito.never()).getNetworkServiceCapabilities(Mockito.anyLong(), Mockito.eq(Service.Firewall)); + } } From 3fc972746cfbc69fb02f8b708110ea58553db8d7 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 12 May 2026 15:29:30 +0530 Subject: [PATCH 08/32] Fix delete firewall flow --- .../network/firewall/FirewallManagerImpl.java | 2 +- systemvm/debian/opt/cloud/bin/configure.py | 51 +++++++++++++++++++ ui/src/views/network/PublicIpResource.vue | 14 +++-- 3 files changed, 61 insertions(+), 6 deletions(-) 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 ca2183f75858..1b164eba92bb 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -912,7 +912,7 @@ public boolean applyRules(List rules, boolean continueOn success = false; } else { removeRule(rule); - if (rule.getSourceIpAddressId() != null) { + if (rule.getSourceIpAddressId() != null && rule.getVpcId() == null) { //if the rule is the last one for the ip address assigned to VPC, unassign it from the network _vpcMgr.unassignIPFromVpcNetwork(rule.getSourceIpAddressId(), rule.getNetworkId()); } diff --git a/systemvm/debian/opt/cloud/bin/configure.py b/systemvm/debian/opt/cloud/bin/configure.py index a250229ba9e6..84c327cfa29a 100755 --- a/systemvm/debian/opt/cloud/bin/configure.py +++ b/systemvm/debian/opt/cloud/bin/configure.py @@ -703,6 +703,7 @@ def process(self): self.add_routing_rules() return + desired_firewall_ips = self._get_desired_vpc_firewall_ips() fw_chains_created = set() for item in self.dbag: if item == "id": @@ -725,6 +726,56 @@ def process(self): fw_chains_created.add(src_ip) self.AclIP(self.dbag[item], self.config).create() + if self.config.is_vpc(): + self._cleanup_removed_vpc_firewall_chains(desired_firewall_ips) + + def _get_desired_vpc_firewall_ips(self): + desired_firewall_ips = set() + if not self.config.is_vpc(): + return desired_firewall_ips + + for item in self.dbag: + if item == "id": + continue + rule = self.dbag[item] + if rule.get("purpose") == "Firewall": + src_ip = rule.get("src_ip") + if src_ip: + desired_firewall_ips.add(src_ip) + return desired_firewall_ips + + def _cleanup_removed_vpc_firewall_chains(self, desired_firewall_ips): + """Delete FIREWALL_ chain only when no firewall rule remains for that VPC public IP.""" + try: + mangle_save = CsHelper.execute("iptables-save -t mangle") + existing_firewall_ips = [] + for line in mangle_save: + if line.startswith(":FIREWALL_"): + chain = line.split(" ")[0][1:] + existing_firewall_ips.append(chain.replace("FIREWALL_", "", 1)) + + for src_ip in existing_firewall_ips: + if src_ip in desired_firewall_ips: + continue + self._delete_vpc_firewall_chain(src_ip) + except Exception as e: + logging.debug("Failed VPC firewall chain cleanup: %s", e) + + def _delete_vpc_firewall_chain(self, src_ip): + chain = "FIREWALL_%s" % src_ip + try: + prerouting_rules = CsHelper.execute("iptables -t mangle -S PREROUTING") + for rule in prerouting_rules: + if ("-d %s/32" % src_ip) in rule and ("-j %s" % chain) in rule: + delete_rule = rule.replace("-A PREROUTING", "-D PREROUTING", 1) + CsHelper.execute2("iptables -t mangle %s" % delete_rule, False) + + CsHelper.execute2("iptables -t mangle -F %s" % chain, False) + CsHelper.execute2("iptables -t mangle -X %s" % chain, False) + logging.info("Deleted VPC firewall chain %s as last firewall rule was removed", chain) + except Exception as e: + logging.debug("Failed deleting VPC firewall chain %s: %s", chain, e) + class CsIpv6Firewall(CsDataBag): """ Deal with IPv6 Firewall diff --git a/ui/src/views/network/PublicIpResource.vue b/ui/src/views/network/PublicIpResource.vue index 5a5ea99a0031..0f77cb52937a 100644 --- a/ui/src/views/network/PublicIpResource.vue +++ b/ui/src/views/network/PublicIpResource.vue @@ -147,11 +147,15 @@ export default { // VPC IPs with static nat keep existing VPN behavior and always show firewall if (this.resource.isstaticnat) { - let tabs = this.defaultTabs - if (this.resource.virtualmachinetype === 'DomainRouter') { - tabs = this.defaultTabs.concat(this.$route.meta.tabs.filter(tab => tab.name === 'vpn')) - } - this.tabs = hasFirewallCapability ? this.addFirewallTab(tabs) : tabs + const tabs = this.addFirewallTab(this.$route.meta.tabs).map(tab => { + if (tab.name !== 'firewall') { + return tab + } + const staticNatFirewallTab = { ...tab } + delete staticNatFirewallTab.networkServiceFilter + return staticNatFirewallTab + }) + this.tabs = tabs return } From 47a15da638160bcf56e85dd5cfe7e785c3d6c576 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 12 May 2026 16:01:50 +0530 Subject: [PATCH 09/32] remove default offering changes --- .../upgrade/dao/Upgrade42210to42300.java | 64 ------------------- 1 file changed, 64 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42300.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42300.java index a8e5365e5c10..393f20399503 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42300.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42300.java @@ -17,8 +17,6 @@ package com.cloud.upgrade.dao; import java.io.InputStream; -import java.util.Arrays; -import java.util.List; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -53,7 +51,6 @@ public InputStream[] getPrepareScripts() { @Override public void performDataMigration(Connection conn) { unhideJsInterpretationEnabled(conn); - updateVpcDefaultOfferingsWithFirewallService(conn); } protected void unhideJsInterpretationEnabled(Connection conn) { @@ -92,65 +89,4 @@ protected void updateJsInterpretationEnabledFields(Connection conn, String encry logger.warn("Error while decrypting configuration 'js.interpretation.enabled'. The configuration may already be decrypted."); } } - - private void updateVpcDefaultOfferingsWithFirewallService(Connection conn) { - logger.debug("Updating default VPC offerings to add Firewall service with VpcVirtualRouter provider"); - - final List defaultVpcOfferingUniqueNames = Arrays.asList( - "DefaultIsolatedNetworkOfferingForVpcNetworks", - "DefaultIsolatedNetworkOfferingForVpcNetworksNoLB", - "DefaultIsolatedNetworkOfferingForVpcNetworksWithInternalLB", - "DefaultNATNSXNetworkOfferingForVpc", - "DefaultRoutedNSXNetworkOfferingForVpc", - "DefaultNATNSXNetworkOfferingForVpcWithInternalLB", - "DefaultRoutedNetrisNetworkOfferingForVpc", - "DefaultNATNetrisNetworkOfferingForVpc", - "DefaultNSXVPCNetworkOfferingforKubernetesService" - ); - - try { - for (String uniqueName : defaultVpcOfferingUniqueNames) { - PreparedStatement pstmt = conn.prepareStatement("SELECT id FROM `cloud`.`network_offerings` WHERE unique_name = ?"); - pstmt.setString(1, uniqueName); - - ResultSet rs = pstmt.executeQuery(); - if (!rs.next()) { - continue; - } - - long offeringId = rs.getLong(1); - rs.close(); - pstmt.close(); - - // Insert into ntwk_offering_service_map (if not exists) - pstmt = conn.prepareStatement("INSERT INTO `cloud`.`ntwk_offering_service_map` " + - "(network_offering_id, service, provider, created) " + - "VALUES (?, 'Firewall', 'VpcVirtualRouter', now())"); - pstmt.setLong(1, offeringId); - pstmt.executeUpdate(); - pstmt.close(); - - // Update existing networks (ntwk_service_map) - pstmt = conn.prepareStatement("SELECT id FROM `cloud`.`networks` WHERE network_offering_id = ?"); - pstmt.setLong(1, offeringId); - - rs = pstmt.executeQuery(); - while (rs.next()) { - long networkId = rs.getLong(1); - PreparedStatement insertService = conn.prepareStatement("INSERT INTO `cloud`.`ntwk_service_map` " + - "(network_id, service, provider, created) " + - "VALUES (?, 'Firewall', 'VpcVirtualRouter', now())"); - insertService.setLong(1, networkId); - insertService.executeUpdate(); - insertService.close(); - } - - rs.close(); - pstmt.close(); - } - - } catch (SQLException e) { - logger.warn("Exception while updating VPC default offerings with Firewall service: " + e.getMessage(), e); - } - } } From 04fb32382a833cd0ab790aa7eb1372de958b78bb Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 12 May 2026 16:04:47 +0530 Subject: [PATCH 10/32] Fix unit test --- .../kubernetes/cluster/KubernetesClusterManagerImplTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java index 71949459c865..1fab5420c3c3 100644 --- a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java +++ b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java @@ -155,7 +155,7 @@ public void validateIsolatedNetworkIpRulesNoRules() { } private FirewallRuleVO createRule(int startPort, int endPort) { - FirewallRuleVO rule = new FirewallRuleVO(null, null, startPort, endPort, "tcp", 1, 1, 1, FirewallRule.Purpose.Firewall, List.of("0.0.0.0/0"), null, null, null, FirewallRule.TrafficType.Ingress); + FirewallRuleVO rule = new FirewallRuleVO(null, null, startPort, endPort, "tcp", 1L, 1, 1, FirewallRule.Purpose.Firewall, List.of("0.0.0.0/0"), null, null, null, FirewallRule.TrafficType.Ingress); return rule; } From 69d6698a4bca1a5a94a31e9521b121dc6177feb1 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 12 May 2026 16:13:27 +0530 Subject: [PATCH 11/32] Fix whitespace --- systemvm/debian/opt/cloud/bin/configure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/systemvm/debian/opt/cloud/bin/configure.py b/systemvm/debian/opt/cloud/bin/configure.py index 84c327cfa29a..5d4085b091cb 100755 --- a/systemvm/debian/opt/cloud/bin/configure.py +++ b/systemvm/debian/opt/cloud/bin/configure.py @@ -707,7 +707,7 @@ def process(self): fw_chains_created = set() for item in self.dbag: if item == "id": - continue + continue if self.config.is_vpc() and not ("purpose" in self.dbag[item] and self.dbag[item]["purpose"] == "Firewall"): self.AclDevice(self.dbag[item], self.config).create() else: From d4f06db7fd82d92f220030e1ae816e299277c73c Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 12 May 2026 16:18:08 +0530 Subject: [PATCH 12/32] remove Unnecessary Stubbing --- .../java/com/cloud/network/firewall/FirewallManagerTest.java | 2 -- 1 file changed, 2 deletions(-) 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 e570880545e9..c33af926e810 100644 --- a/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java +++ b/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java @@ -357,11 +357,9 @@ public void testValidateFirewallRuleVpcWithoutAssociatedNetworkUsesVpcCapabiliti firewallCaps.put(Capability.SupportedEgressProtocols, "tcp,udp,icmp"); capabilities.put(Service.Firewall, firewallCaps); - when(ipAddress.getAssociatedWithNetworkId()).thenReturn(null); when(ipAddress.getVpcId()).thenReturn(10L); when(_networkModel.getNetwork(2L)).thenReturn(network); when(network.getVpcId()).thenReturn(10L); - when(network.getDataCenterId()).thenReturn(1L); when(routedIpv4Manager.isVirtualRouterGateway(network)).thenReturn(false); when(firewallServiceProvider.getProvider()).thenReturn(Network.Provider.VPCVirtualRouter); when(firewallServiceProvider.getCapabilities()).thenReturn(capabilities); From 6ec02982be033e3a6a861cd1dd2c8df73ed4bf79 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 12 May 2026 17:21:00 +0530 Subject: [PATCH 13/32] more unit test fixes --- .../java/com/cloud/network/resource/PaloAltoResourceTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/network-elements/palo-alto/src/test/java/com/cloud/network/resource/PaloAltoResourceTest.java b/plugins/network-elements/palo-alto/src/test/java/com/cloud/network/resource/PaloAltoResourceTest.java index 58f962f13727..ae8520e005a7 100644 --- a/plugins/network-elements/palo-alto/src/test/java/com/cloud/network/resource/PaloAltoResourceTest.java +++ b/plugins/network-elements/palo-alto/src/test/java/com/cloud/network/resource/PaloAltoResourceTest.java @@ -290,7 +290,7 @@ public void addEgressFirewallRule() throws ConfigurationException, Exception { List rules = new ArrayList(); List cidrList = new ArrayList(); cidrList.add("0.0.0.0/0"); - FirewallRuleVO activeVO = new FirewallRuleVO(null, null, 80, 80, "tcp", 1, 1, 1, Purpose.Firewall, cidrList, null, null, null, FirewallRule.TrafficType.Egress); + FirewallRuleVO activeVO = new FirewallRuleVO(null, null, 80, 80, "tcp", 1L, 1, 1, Purpose.Firewall, cidrList, null, null, null, FirewallRule.TrafficType.Egress); FirewallRuleTO active = new FirewallRuleTO(activeVO, Long.toString(vlanId), null, Purpose.Firewall, FirewallRule.TrafficType.Egress); rules.add(active); @@ -319,7 +319,7 @@ public void removeEgressFirewallRule() throws ConfigurationException, Exception long vlanId = 3954; List rules = new ArrayList(); - FirewallRuleVO revokedVO = new FirewallRuleVO(null, null, 80, 80, "tcp", 1, 1, 1, Purpose.Firewall, null, null, null, null, FirewallRule.TrafficType.Egress); + FirewallRuleVO revokedVO = new FirewallRuleVO(null, null, 80, 80, "tcp", 1L, 1, 1, Purpose.Firewall, null, null, null, null, FirewallRule.TrafficType.Egress); revokedVO.setState(State.Revoke); FirewallRuleTO revoked = new FirewallRuleTO(revokedVO, Long.toString(vlanId), null, Purpose.Firewall, FirewallRule.TrafficType.Egress); rules.add(revoked); From 84e44e8bfc3f0a9591b194e4120df099406e017f Mon Sep 17 00:00:00 2001 From: Harikrishna Date: Wed, 13 May 2026 12:17:59 +0530 Subject: [PATCH 14/32] Update server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java Co-authored-by: Wei Zhou --- .../org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java index 07f6346a8786..d49b25e216b3 100644 --- a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java @@ -989,7 +989,8 @@ public boolean applyRoutingFirewallRule(long id) { @Override public boolean isVirtualRouterGateway(Network network) { return isRoutedNetwork(network) - && (networkServiceMapDao.canProviderSupportServiceInNetwork(network.getId(), Service.Gateway, Provider.VirtualRouter)); + && (networkServiceMapDao.canProviderSupportServiceInNetwork(network.getId(), Service.Gateway, Provider.VirtualRouter) + || networkServiceMapDao.canProviderSupportServiceInNetwork(network.getId(), Service.Gateway, Provider.VPCVirtualRouter)); } @Override From 7ffa4f085d8ca8189d48f4846441130ba71a2684 Mon Sep 17 00:00:00 2001 From: Harikrishna Date: Wed, 13 May 2026 12:18:16 +0530 Subject: [PATCH 15/32] Update server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java Co-authored-by: Wei Zhou --- .../org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java index d49b25e216b3..213d8ee0087b 100644 --- a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java @@ -996,7 +996,8 @@ public boolean isVirtualRouterGateway(Network network) { @Override public boolean isVirtualRouterGateway(NetworkOffering networkOffering) { return NetworkOffering.NetworkMode.ROUTED.equals(networkOffering.getNetworkMode()) - && networkOfferingServiceMapDao.canProviderSupportServiceInNetworkOffering(networkOffering.getId(), Service.Gateway, Provider.VirtualRouter); + && (networkOfferingServiceMapDao.canProviderSupportServiceInNetworkOffering(networkOffering.getId(), Service.Gateway, Provider.VirtualRouter) + || networkOfferingServiceMapDao.canProviderSupportServiceInNetworkOffering(networkOffering.getId(), Service.Gateway, Provider.VPCVirtualRouter)); } @Override From 613d190859cef20cd4983ed86af693bdf5f237d9 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Wed, 13 May 2026 12:19:16 +0530 Subject: [PATCH 16/32] DB and response changes --- .../api/response/FirewallResponse.java | 8 +++++ .../META-INF/db/schema-42210to42300.sql | 3 ++ .../java/com/cloud/api/ApiResponseHelper.java | 31 +++++++++++++++++-- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/response/FirewallResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/FirewallResponse.java index 5986c16dc8c0..f6cc9e5d9499 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/FirewallResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/FirewallResponse.java @@ -51,6 +51,10 @@ public class FirewallResponse extends BaseResponse { @Param(description = "The Network ID of the firewall rule") private String networkId; + @SerializedName(ApiConstants.VPC_ID) + @Param(description = "The VPC ID of the firewall rule") + private String vpcId; + @SerializedName(ApiConstants.IP_ADDRESS) @Param(description = "The public IP address for the firewall rule") private String publicIpAddress; @@ -115,6 +119,10 @@ public void setNetworkId(String networkId) { this.networkId = networkId; } + public void setVpcId(String vpcId) { + this.vpcId = vpcId; + } + public void setState(String state) { this.state = state; } diff --git a/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql b/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql index 3240c431fc83..80293aaab353 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql @@ -588,3 +588,6 @@ CREATE TABLE IF NOT EXISTS `cloud`.`dns_zone_network_map` ( CONSTRAINT `fk_dns_map__zone_id` FOREIGN KEY (`dns_zone_id`) REFERENCES `dns_zone` (`id`) ON DELETE CASCADE, CONSTRAINT `fk_dns_map__network_id` FOREIGN KEY (`network_id`) REFERENCES `networks` (`id`) ON DELETE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- This is part of allowing firewall rules on public IP addresses in VPC network +ALTER TABLE `cloud`.`firewall_rules` MODIFY COLUMN `network_id` BIGINT UNSIGNED NULL; diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index 3a0d4cecc78d..b0f8711d5380 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -2976,6 +2976,14 @@ public FirewallResponse createFirewallResponse(FirewallRule fwRule) { } } + Long vpcId = fwRule.getVpcId(); + if (vpcId != null) { + Vpc vpc = ApiDBUtils.findVpcById(vpcId); + if (vpc != null) { + response.setVpcId(vpc.getUuid()); + } + } + FirewallRule.State state = fwRule.getState(); String stateToSet = state.toString(); if (state.equals(FirewallRule.State.Revoke)) { @@ -5425,8 +5433,27 @@ public FirewallResponse createIpv6FirewallRuleResponse(FirewallRule fwRule) { response.setIcmpCode(fwRule.getIcmpCode()); response.setIcmpType(fwRule.getIcmpType()); - Network network = ApiDBUtils.findNetworkById(fwRule.getNetworkId()); - response.setNetworkId(network.getUuid()); + Long networkId = fwRule.getNetworkId(); + if (networkId != null) { + Network network = ApiDBUtils.findNetworkById(networkId); + if (network != null) { + response.setNetworkId(network.getUuid()); + } + } + + Long vpcId = fwRule.getVpcId(); + if (vpcId == null && networkId != null) { + Network network = ApiDBUtils.findNetworkById(networkId); + if (network != null) { + vpcId = network.getVpcId(); + } + } + if (vpcId != null) { + Vpc vpc = ApiDBUtils.findVpcById(vpcId); + if (vpc != null) { + response.setVpcId(vpc.getUuid()); + } + } FirewallRule.State state = fwRule.getState(); String stateToSet = state.toString(); From 15e67dec61c5242f09ef7c3099ca93be74ac80e9 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Wed, 13 May 2026 12:32:20 +0530 Subject: [PATCH 17/32] marvin test --- .../smoke/test_vpc_firewall_rules.py | 183 ++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 test/integration/smoke/test_vpc_firewall_rules.py diff --git a/test/integration/smoke/test_vpc_firewall_rules.py b/test/integration/smoke/test_vpc_firewall_rules.py new file mode 100644 index 000000000000..36acfaa1b345 --- /dev/null +++ b/test/integration/smoke/test_vpc_firewall_rules.py @@ -0,0 +1,183 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Smoke tests for firewall rules on VPC public IPs.""" + +from nose.plugins.attrib import attr + +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.base import Account, FireWallRule, Network, NetworkOffering, PublicIPAddress, VPC, VpcOffering +from marvin.lib.common import get_domain, get_zone, list_publicIP +from marvin.lib.utils import cleanup_resources, wait_until + + +class TestVpcFirewallRules(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + cls.testClient = super(TestVpcFirewallRules, cls).getClsTestClient() + cls.apiclient = cls.testClient.getApiClient() + cls.services = cls.testClient.getParsedTestDataConfig() + cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests()) + cls.domain = get_domain(cls.apiclient) + cls._cleanup = [] + + cls.account = Account.create( + cls.apiclient, + cls.services["account"], + domainid=cls.domain.id + ) + cls._cleanup.append(cls.account) + + cls.services["vpc_offering"]["supportedservices"] = "Vpn,Dhcp,Dns,SourceNat,Lb,UserData,StaticNat,NetworkACL,PortForwarding,Firewall" + cls.services["vpc_offering"]["serviceProviderList"] = { + "Vpn": "VpcVirtualRouter", + "Dhcp": "VpcVirtualRouter", + "Dns": "VpcVirtualRouter", + "SourceNat": "VpcVirtualRouter", + "Lb": "VpcVirtualRouter", + "UserData": "VpcVirtualRouter", + "StaticNat": "VpcVirtualRouter", + "NetworkACL": "VpcVirtualRouter", + "PortForwarding": "VpcVirtualRouter", + "Firewall": "VpcVirtualRouter" + } + + cls.vpc_offering = VpcOffering.create( + cls.apiclient, + cls.services["vpc_offering"] + ) + cls.vpc_offering.update(cls.apiclient, state="Enabled") + cls._cleanup.append(cls.vpc_offering) + + network_offering = NetworkOffering.list( + cls.apiclient, + name="DefaultIsolatedNetworkOfferingForVpcNetworks" + ) + cls.assertTrue(network_offering is not None and len(network_offering) > 0, + "No VPC tier network offering found") + cls.network_offering = network_offering[0] + + cls.services["vpc"]["cidr"] = "10.20.30.0/24" + cls.vpc = VPC.create( + cls.apiclient, + cls.services["vpc"], + vpcofferingid=cls.vpc_offering.id, + zoneid=cls.zone.id, + account=cls.account.name, + domainid=cls.account.domainid + ) + cls._cleanup.append(cls.vpc) + + cls.tier = Network.create( + cls.apiclient, + services={"name": "vpc-fw-tier", "displaytext": "vpc-fw-tier"}, + accountid=cls.account.name, + domainid=cls.account.domainid, + networkofferingid=cls.network_offering.id, + zoneid=cls.zone.id, + vpcid=cls.vpc.id, + gateway="10.20.30.1", + netmask="255.255.255.0" + ) + cls._cleanup.append(cls.tier) + + @classmethod + def tearDownClass(cls): + try: + cleanup_resources(cls.apiclient, cls._cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup: %s" % e) + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.cleanup = [] + + def tearDown(self): + cleanup_resources(self.apiclient, self.cleanup) + + def _wait_for_firewall_rule(self, rule_id): + rules = FireWallRule.list(self.apiclient, id=rule_id, listall=True) + if rules and len(rules) == 1: + return True, rules[0] + return False, None + + @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="false") + def test_01_create_firewall_rule_on_vpc_public_ip(self): + """Verify firewall rule can be created and listed on a dedicated VPC public IP.""" + public_ip = PublicIPAddress.create( + self.apiclient, + zoneid=self.zone.id, + accountid=self.account.name, + domainid=self.account.domainid, + vpcid=self.vpc.id + ) + self.cleanup.append(public_ip) + + firewall_rule = FireWallRule.create( + self.apiclient, + ipaddressid=public_ip.ipaddress.id, + protocol="tcp", + cidrlist=["0.0.0.0/0"], + startport=19090, + endport=19090, + vpcid=self.vpc.id + ) + self.cleanup.insert(0, firewall_rule) + + result, listed_rule = wait_until(2, 10, self._wait_for_firewall_rule, firewall_rule.id) + self.assertTrue(result, "Firewall rule was not listed for the VPC public IP") + self.assertEqual(listed_rule.id, firewall_rule.id) + self.assertEqual(listed_rule.ipaddressid, public_ip.ipaddress.id) + self.assertEqual(listed_rule.vpcid, self.vpc.id) + self.assertEqual(listed_rule.protocol.lower(), "tcp") + self.assertEqual(int(listed_rule.startport), 19090) + self.assertEqual(int(listed_rule.endport), 19090) + + @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="false") + def test_02_create_firewall_rule_on_vpc_source_nat_ip(self): + """Verify firewall rule can be created and listed on the VPC source NAT IP.""" + source_nat_ips = list_publicIP( + self.apiclient, + vpcid=self.vpc.id, + listall=True, + issourcenat=True + ) + self.assertTrue(source_nat_ips is not None and len(source_nat_ips) > 0, + "No source NAT IP found for the VPC") + source_nat_ip = source_nat_ips[0] + + firewall_rule = FireWallRule.create( + self.apiclient, + ipaddressid=source_nat_ip.id, + protocol="tcp", + cidrlist=["0.0.0.0/0"], + startport=19443, + endport=19443, + vpcid=self.vpc.id + ) + self.cleanup.append(firewall_rule) + + result, listed_rule = wait_until(2, 10, self._wait_for_firewall_rule, firewall_rule.id) + self.assertTrue(result, "Firewall rule was not listed for the VPC source NAT IP") + self.assertEqual(listed_rule.id, firewall_rule.id) + self.assertEqual(listed_rule.ipaddressid, source_nat_ip.id) + self.assertEqual(listed_rule.vpcid, self.vpc.id) + self.assertEqual(listed_rule.protocol.lower(), "tcp") + self.assertEqual(int(listed_rule.startport), 19443) + self.assertEqual(int(listed_rule.endport), 19443) + From 9d717047c4809cf9cece8f5b1cc2c828fd13a0bf Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Wed, 13 May 2026 13:20:59 +0530 Subject: [PATCH 18/32] more unit tests --- .../cloud/network/IpAddressManagerTest.java | 175 +++++++++ .../network/firewall/FirewallManagerTest.java | 343 ++++++++++++++++++ 2 files changed, 518 insertions(+) diff --git a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java index cf3a886ce99f..c7b6e851925c 100644 --- a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java +++ b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java @@ -19,10 +19,13 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -54,8 +57,12 @@ import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; +import com.cloud.network.rules.FirewallRule; import com.cloud.network.rules.StaticNat; import com.cloud.network.rules.StaticNatImpl; +import com.cloud.network.vpc.VpcManager; +import com.cloud.dc.dao.VlanDao; +import com.cloud.dc.VlanVO; import com.cloud.offerings.NetworkOfferingVO; import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.user.AccountVO; @@ -105,6 +112,12 @@ public class IpAddressManagerTest { @Mock AccountManager accountManagerMock; + @Mock + VpcManager vpcMgr; + + @Mock + VlanDao vlanDao; + final long dummyID = 1L; final String UUID = "uuid"; @@ -492,4 +505,166 @@ public void checkIfIpResourceCountShouldBeUpdatedTestIpIsAssociatedToVpcAndNotDe Assert.assertTrue(result); } + + + private FirewallRule makeRule(Long networkId, Long vpcId, FirewallRule.Purpose purpose, + FirewallRule.TrafficType trafficType) { + FirewallRule rule = mock(FirewallRule.class); + lenient().when(rule.getNetworkId()).thenReturn(networkId); + lenient().when(rule.getVpcId()).thenReturn(vpcId); + lenient().when(rule.getPurpose()).thenReturn(purpose); + lenient().when(rule.getTrafficType()).thenReturn(trafficType); + return rule; + } + + /** Stub the two IP-association helper methods so they are no-ops. */ + private void stubIpAssocHelpers() throws ResourceUnavailableException { + doReturn(false).when(ipAddressManager).checkIfIpAssocRequired(any(Network.class), anyBoolean(), any()); + } + + /** + * Test: Non-VPC rules still resolve via networkId (backward compatibility). + */ + @Test + public void applyRulesNonVpcRuleStillWorksViaNetworkId() throws ResourceUnavailableException { + long networkId = 10L; + NetworkVO network = mock(NetworkVO.class); + when(network.getId()).thenReturn(networkId); + when(networkDao.findById(networkId)).thenReturn(network); + + FirewallRule rule = makeRule(networkId, null, FirewallRule.Purpose.Firewall, FirewallRule.TrafficType.Ingress); + NetworkRuleApplier applier = mock(NetworkRuleApplier.class); + + when(ipAddressDao.listByAssociatedNetwork(networkId, null)).thenReturn(new ArrayList<>()); + stubIpAssocHelpers(); + + boolean result = ipAddressManager.applyRules( + Collections.singletonList(rule), FirewallRule.Purpose.Firewall, applier, false); + + assertTrue(result); + verify(networkDao).findById(networkId); + verify(applier).applyRules(network, FirewallRule.Purpose.Firewall, Collections.singletonList(rule)); + } + + /** + * Test: VPC rule resolves network via VpcManager.getVpcNetworks() + * when networkId is null but vpcId is set. + */ + @Test + public void applyRulesVpcRuleResolvesNetworkViaVpcManager() throws ResourceUnavailableException { + long vpcId = 20L; + long resolvedNetworkId = 30L; + NetworkVO resolvedNetwork = mock(NetworkVO.class); + when(resolvedNetwork.getId()).thenReturn(resolvedNetworkId); + when(networkDao.findById(resolvedNetworkId)).thenReturn(resolvedNetwork); + + doReturn(Collections.singletonList(resolvedNetwork)).when(vpcMgr).getVpcNetworks(vpcId); + + FirewallRule rule = makeRule(null, vpcId, FirewallRule.Purpose.Firewall, FirewallRule.TrafficType.Ingress); + NetworkRuleApplier applier = mock(NetworkRuleApplier.class); + + IPAddressVO vpcIp = mock(IPAddressVO.class); + when(vpcIp.getVlanId()).thenReturn(1L); + VlanVO vlan = mock(VlanVO.class); + when(ipAddressDao.listByAssociatedVpc(vpcId, null)).thenReturn(Collections.singletonList(vpcIp)); + when(vlanDao.findById(1L)).thenReturn(vlan); + + stubIpAssocHelpers(); + + boolean result = ipAddressManager.applyRules( + Collections.singletonList(rule), FirewallRule.Purpose.Firewall, applier, false); + + assertTrue(result); + verify(vpcMgr).getVpcNetworks(vpcId); + verify(ipAddressDao).listByAssociatedVpc(vpcId, null); + verify(applier).applyRules(resolvedNetwork, FirewallRule.Purpose.Firewall, Collections.singletonList(rule)); + } + + + /** + * Test: For VPC egress firewall rules, IP collection should be skipped. + */ + @Test + public void applyRulesVpcEgressFirewallRuleSkipsIpCollection() throws ResourceUnavailableException { + long vpcId = 20L; + long resolvedNetworkId = 30L; + NetworkVO resolvedNetwork = mock(NetworkVO.class); + when(resolvedNetwork.getId()).thenReturn(resolvedNetworkId); + when(networkDao.findById(resolvedNetworkId)).thenReturn(resolvedNetwork); + doReturn(Collections.singletonList(resolvedNetwork)).when(vpcMgr).getVpcNetworks(vpcId); + + FirewallRule rule = makeRule(null, vpcId, FirewallRule.Purpose.Firewall, FirewallRule.TrafficType.Egress); + NetworkRuleApplier applier = mock(NetworkRuleApplier.class); + + stubIpAssocHelpers(); + + boolean result = ipAddressManager.applyRules( + Collections.singletonList(rule), FirewallRule.Purpose.Firewall, applier, false); + + assertTrue(result); + verify(ipAddressDao, never()).listByAssociatedVpc(anyLong(), any()); + verify(applier).applyRules(resolvedNetwork, FirewallRule.Purpose.Firewall, Collections.singletonList(rule)); + } + + /** + * Test: VPC ingress firewall rules collect public IPs from VPC (listByAssociatedVpc), + * NOT from network (listByAssociatedNetwork). + */ + @Test + public void applyRulesVpcIngressRuleCollectsIpsFromVpcNotNetwork() throws ResourceUnavailableException { + long vpcId = 20L; + long resolvedNetworkId = 30L; + NetworkVO resolvedNetwork = mock(NetworkVO.class); + when(resolvedNetwork.getId()).thenReturn(resolvedNetworkId); + when(networkDao.findById(resolvedNetworkId)).thenReturn(resolvedNetwork); + doReturn(Collections.singletonList(resolvedNetwork)).when(vpcMgr).getVpcNetworks(vpcId); + + IPAddressVO vpcIp = mock(IPAddressVO.class); + when(vpcIp.getVlanId()).thenReturn(1L); + VlanVO vlan = mock(VlanVO.class); + when(ipAddressDao.listByAssociatedVpc(vpcId, null)).thenReturn(Collections.singletonList(vpcIp)); + when(vlanDao.findById(1L)).thenReturn(vlan); + + stubIpAssocHelpers(); + + NetworkRuleApplier applier = mock(NetworkRuleApplier.class); + FirewallRule rule = makeRule(null, vpcId, FirewallRule.Purpose.Firewall, FirewallRule.TrafficType.Ingress); + + ipAddressManager.applyRules(Collections.singletonList(rule), FirewallRule.Purpose.Firewall, applier, false); + + verify(ipAddressDao).listByAssociatedVpc(vpcId, null); + verify(ipAddressDao, never()).listByAssociatedNetwork(anyLong(), any()); + } + + /** + * Test: Error handling respects continueOnError flag. + * When continueOnError=true, exceptions are caught and false is returned. + */ + @Test + public void applyRulesVpcRuleErrorHandlingWithContinueOnErrorTrue() throws ResourceUnavailableException { + long vpcId = 20L; + long resolvedNetworkId = 30L; + NetworkVO resolvedNetwork = mock(NetworkVO.class); + when(resolvedNetwork.getId()).thenReturn(resolvedNetworkId); + when(networkDao.findById(resolvedNetworkId)).thenReturn(resolvedNetwork); + doReturn(Collections.singletonList(resolvedNetwork)).when(vpcMgr).getVpcNetworks(vpcId); + + IPAddressVO vpcIp = mock(IPAddressVO.class); + when(vpcIp.getVlanId()).thenReturn(1L); + VlanVO vlan = mock(VlanVO.class); + when(ipAddressDao.listByAssociatedVpc(vpcId, null)).thenReturn(Collections.singletonList(vpcIp)); + when(vlanDao.findById(1L)).thenReturn(vlan); + + stubIpAssocHelpers(); + + NetworkRuleApplier applier = mock(NetworkRuleApplier.class); + when(applier.applyRules(any(), any(), any())).thenThrow(new ResourceUnavailableException("test", Network.class, 0L)); + + FirewallRule rule = makeRule(null, vpcId, FirewallRule.Purpose.Firewall, FirewallRule.TrafficType.Ingress); + + boolean result = ipAddressManager.applyRules( + Collections.singletonList(rule), FirewallRule.Purpose.Firewall, applier, true); + + assertFalse(result); + } } 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 c33af926e810..5ef60c699b2f 100644 --- a/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java +++ b/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java @@ -17,6 +17,7 @@ package com.cloud.network.firewall; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.NetworkRuleConflictException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.network.IpAddressManager; @@ -59,14 +60,18 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -370,4 +375,342 @@ public void testValidateFirewallRuleVpcWithoutAssociatedNetworkUsesVpcCapabiliti verify(_networkModel, Mockito.never()).getNetworkServiceCapabilities(Mockito.anyLong(), Mockito.eq(Service.Firewall)); } + + @Test + public void testIsVpcIpAddressReturnsTrueWhenVpcIdPresent() { + IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); + when(ipAddress.getVpcId()).thenReturn(5L); + Assert.assertTrue(_firewallMgr.isVpcIpAddress(ipAddress)); + } + + @Test + public void testIsVpcIpAddressReturnsFalseWhenVpcIdNull() { + IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); + when(ipAddress.getVpcId()).thenReturn(null); + Assert.assertFalse(_firewallMgr.isVpcIpAddress(ipAddress)); + } + + @Test + public void testValidateFirewallRuleForIsolatedIpReturnsNetworkId() { + IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); + when(ipAddress.getAssociatedWithNetworkId()).thenReturn(42L); + Long result = _firewallMgr.validateFirewallRuleForIsolatedIp(ipAddress); + Assert.assertEquals(Long.valueOf(42L), result); + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateFirewallRuleForIsolatedIpThrowsWhenNotAssociated() { + IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); + when(ipAddress.getAssociatedWithNetworkId()).thenReturn(null); + _firewallMgr.validateFirewallRuleForIsolatedIp(ipAddress); + } + + @Test + public void testValidateFirewallRuleForVpcIpReturnsNetworkId() { + IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); + Long result = _firewallMgr.validateFirewallRuleForVpcIp(ipAddress, 99L); + Assert.assertEquals(Long.valueOf(99L), result); + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateFirewallRuleForVpcIpThrowsWhenNetworkIdNull() { + IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); + _firewallMgr.validateFirewallRuleForVpcIp(ipAddress, null); + } + + @Test + public void testGetFirewallServiceCapabilitiesForNonVpcNetworkUsesNetworkModel() { + NetworkVO network = Mockito.mock(NetworkVO.class); + when(network.getId()).thenReturn(1L); + when(network.getVpcId()).thenReturn(null); + Map caps = new HashMap<>(); + caps.put(Capability.SupportedProtocols, "tcp,udp"); + when(_networkModel.getNetworkServiceCapabilities(1L, Service.Firewall)).thenReturn(caps); + + Map result = _firewallMgr.getFirewallServiceCapabilities(network); + + Assert.assertEquals(caps, result); + verify(_networkModel, times(1)).getNetworkServiceCapabilities(1L, Service.Firewall); + } + + @Test + public void testGetFirewallServiceCapabilitiesForVpcNetworkUsesVpcProvider() { + NetworkVO network = Mockito.mock(NetworkVO.class); + FirewallServiceProvider fwProvider = Mockito.mock(FirewallServiceProvider.class); + Map firewallCaps = new HashMap<>(); + firewallCaps.put(Capability.SupportedProtocols, "tcp,udp,icmp"); + Map> providerCapabilities = new HashMap<>(); + providerCapabilities.put(Service.Firewall, firewallCaps); + + when(network.getId()).thenReturn(1L); + when(network.getVpcId()).thenReturn(10L); + when(fwProvider.getProvider()).thenReturn(Network.Provider.VPCVirtualRouter); + when(fwProvider.getCapabilities()).thenReturn(providerCapabilities); + when(_vpcMgr.isProviderSupportServiceInVpc(10L, Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(true); + _firewallMgr._firewallElements = List.of(fwProvider); + + Map result = _firewallMgr.getFirewallServiceCapabilities(network); + + Assert.assertEquals(firewallCaps, result); + verify(_networkModel, never()).getNetworkServiceCapabilities(Mockito.anyLong(), Mockito.eq(Service.Firewall)); + } + + @Test + public void testGetFirewallServiceCapabilitiesForVpcNetworkFallsBackToNetworkModelWhenNoProvider() { + NetworkVO network = Mockito.mock(NetworkVO.class); + FirewallServiceProvider fwProvider = Mockito.mock(FirewallServiceProvider.class); + Map fallbackCaps = new HashMap<>(); + + when(network.getId()).thenReturn(1L); + when(network.getVpcId()).thenReturn(10L); + when(fwProvider.getProvider()).thenReturn(Network.Provider.VPCVirtualRouter); + when(_vpcMgr.isProviderSupportServiceInVpc(10L, Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(false); + when(_networkModel.getNetworkServiceCapabilities(1L, Service.Firewall)).thenReturn(fallbackCaps); + _firewallMgr._firewallElements = List.of(fwProvider); + + Map result = _firewallMgr.getFirewallServiceCapabilities(network); + + Assert.assertEquals(fallbackCaps, result); + verify(_networkModel, times(1)).getNetworkServiceCapabilities(1L, Service.Firewall); + } + + @Test + public void testGetFirewallServiceCapabilitiesForVpcReturnsCapabilitiesWhenProviderSupports() { + FirewallServiceProvider fwProvider = Mockito.mock(FirewallServiceProvider.class); + Map firewallCaps = new HashMap<>(); + firewallCaps.put(Capability.SupportedProtocols, "tcp,udp"); + Map> caps = new HashMap<>(); + caps.put(Service.Firewall, firewallCaps); + + when(fwProvider.getProvider()).thenReturn(Network.Provider.VPCVirtualRouter); + when(fwProvider.getCapabilities()).thenReturn(caps); + when(_vpcMgr.isProviderSupportServiceInVpc(10L, Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(true); + _firewallMgr._firewallElements = List.of(fwProvider); + + Map result = _firewallMgr.getFirewallServiceCapabilitiesForVpc(10L); + + Assert.assertNotNull(result); + Assert.assertEquals("tcp,udp", result.get(Capability.SupportedProtocols)); + } + + @Test + public void testGetFirewallServiceCapabilitiesForVpcReturnsNullWhenNoProviderSupports() { + FirewallServiceProvider fwProvider = Mockito.mock(FirewallServiceProvider.class); + when(fwProvider.getProvider()).thenReturn(Network.Provider.VPCVirtualRouter); + when(_vpcMgr.isProviderSupportServiceInVpc(10L, Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(false); + _firewallMgr._firewallElements = List.of(fwProvider); + + Map result = _firewallMgr.getFirewallServiceCapabilitiesForVpc(10L); + + Assert.assertNull(result); + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateFirewallRuleForVpcThrowsOnInvalidStartPort() { + Account caller = Mockito.mock(Account.class); + _firewallMgr.validateFirewallRuleForVpc(caller, null, -1, 80, "tcp", Purpose.Firewall, FirewallRuleType.User, 10L, FirewallRule.TrafficType.Ingress); + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateFirewallRuleForVpcThrowsOnInvalidEndPort() { + Account caller = Mockito.mock(Account.class); + _firewallMgr.validateFirewallRuleForVpc(caller, null, 80, 70000, "tcp", Purpose.Firewall, FirewallRuleType.User, 10L, FirewallRule.TrafficType.Ingress); + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateFirewallRuleForVpcThrowsWhenStartPortGreaterThanEndPort() { + Account caller = Mockito.mock(Account.class); + _firewallMgr.validateFirewallRuleForVpc(caller, null, 200, 100, "tcp", Purpose.Firewall, FirewallRuleType.User, 10L, FirewallRule.TrafficType.Ingress); + } + + @Test + public void testValidateFirewallRuleForVpcSystemTypeWithNullIpReturnsEarly() { + // System rule type + null IP should return without further validation + Account caller = Mockito.mock(Account.class); + // Should not throw even though vpcId checks come after this + _firewallMgr.validateFirewallRuleForVpc(caller, null, 80, 80, "tcp", Purpose.Firewall, FirewallRuleType.System, 10L, FirewallRule.TrafficType.Ingress); + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateFirewallRuleForVpcThrowsWhenVpcIdNullAndNotSystemRule() { + Account caller = Mockito.mock(Account.class); + IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); + _firewallMgr.validateFirewallRuleForVpc(caller, ipAddress, 80, 80, "tcp", Purpose.Firewall, FirewallRuleType.User, null, FirewallRule.TrafficType.Ingress); + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateFirewallRuleForVpcThrowsWhenActiveVpcNotFound() { + Account caller = Mockito.mock(Account.class); + IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); + when(_vpcMgr.getActiveVpc(10L)).thenReturn(null); + _firewallMgr.validateFirewallRuleForVpc(caller, ipAddress, 80, 80, "tcp", Purpose.Firewall, FirewallRuleType.User, 10L, FirewallRule.TrafficType.Ingress); + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateFirewallRuleForVpcThrowsOnUnsupportedProtocol() { + Account caller = Mockito.mock(Account.class); + IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); + Vpc vpc = Mockito.mock(Vpc.class); + FirewallServiceProvider fwProvider = Mockito.mock(FirewallServiceProvider.class); + Map firewallCaps = new HashMap<>(); + firewallCaps.put(Capability.SupportedProtocols, "tcp,udp"); + firewallCaps.put(Capability.SupportedTrafficDirection, "ingress,egress"); + Map> caps = new HashMap<>(); + caps.put(Service.Firewall, firewallCaps); + + when(_vpcMgr.getActiveVpc(10L)).thenReturn(vpc); + when(fwProvider.getProvider()).thenReturn(Network.Provider.VPCVirtualRouter); + when(fwProvider.getCapabilities()).thenReturn(caps); + when(_vpcMgr.isProviderSupportServiceInVpc(10L, Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(true); + _firewallMgr._firewallElements = List.of(fwProvider); + + _firewallMgr.validateFirewallRuleForVpc(caller, ipAddress, 80, 80, "gre", Purpose.Firewall, FirewallRuleType.User, 10L, FirewallRule.TrafficType.Ingress); + } + + @Test + public void testValidateFirewallRuleForVpcSucceedsWithSupportedProtocolAndTrafficType() { + Account caller = Mockito.mock(Account.class); + IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); + Vpc vpc = Mockito.mock(Vpc.class); + FirewallServiceProvider fwProvider = Mockito.mock(FirewallServiceProvider.class); + Map firewallCaps = new HashMap<>(); + firewallCaps.put(Capability.SupportedProtocols, "tcp,udp,icmp"); + firewallCaps.put(Capability.SupportedTrafficDirection, "ingress,egress"); + Map> caps = new HashMap<>(); + caps.put(Service.Firewall, firewallCaps); + + when(_vpcMgr.getActiveVpc(10L)).thenReturn(vpc); + when(fwProvider.getProvider()).thenReturn(Network.Provider.VPCVirtualRouter); + when(fwProvider.getCapabilities()).thenReturn(caps); + when(_vpcMgr.isProviderSupportServiceInVpc(10L, Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(true); + _firewallMgr._firewallElements = List.of(fwProvider); + + // Should not throw + _firewallMgr.validateFirewallRuleForVpc(caller, ipAddress, 80, 80, "tcp", Purpose.Firewall, FirewallRuleType.User, 10L, FirewallRule.TrafficType.Ingress); + + verify(_accountMgr, times(1)).checkAccess(caller, null, true, ipAddress); + } + + @Test + public void testCreateIngressFirewallRuleRoutesToVpcMethodWhenIpHasVpcId() throws NetworkRuleConflictException { + FirewallRule rule = Mockito.mock(FirewallRule.class); + IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); + + when(rule.getSourceIpAddressId()).thenReturn(1L); + when(ipAddress.getVpcId()).thenReturn(10L); + + doReturn(ipAddress).when(_firewallMgr).getSourceIpForIngressRule(1L); + doReturn(rule).when(_firewallMgr).createIngressFirewallRuleForVpcIp(rule, null, ipAddress); + + doReturn(rule).when(_firewallMgr).createIngressFirewallRuleForVpcIp( + Mockito.eq(rule), Mockito.any(), Mockito.eq(ipAddress)); + + verify(_firewallMgr, never()).createIngressFirewallRuleForIsolatedIp( + Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + public void testCreateFirewallRuleRoutesToVpcWhenVpcIdProvided() throws NetworkRuleConflictException { + Account caller = Mockito.mock(Account.class); + FirewallRule vpcRule = Mockito.mock(FirewallRule.class); + + doReturn(vpcRule).when(_firewallMgr).createFirewallRuleForVpc( + Mockito.anyLong(), Mockito.eq(caller), Mockito.any(), Mockito.anyInt(), Mockito.anyInt(), + Mockito.anyString(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(FirewallRuleType.class), Mockito.anyLong(), + Mockito.any(FirewallRule.TrafficType.class), Mockito.anyBoolean()); + + _firewallMgr.createFirewallRule(1L, caller, "xid", 80, 80, "tcp", + Collections.singletonList("0.0.0.0/0"), null, null, null, null, + FirewallRuleType.User, null, 10L, FirewallRule.TrafficType.Ingress, true); + + verify(_firewallMgr, times(1)).createFirewallRuleForVpc( + Mockito.anyLong(), Mockito.eq(caller), Mockito.any(), Mockito.anyInt(), Mockito.anyInt(), + Mockito.anyString(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(FirewallRuleType.class), Mockito.anyLong(), + Mockito.any(FirewallRule.TrafficType.class), Mockito.anyBoolean()); + + verify(_firewallMgr, never()).createFirewallRuleForNonVPC( + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + public void testCreateFirewallRuleRoutesToNonVpcWhenVpcIdNull() throws NetworkRuleConflictException { + Account caller = Mockito.mock(Account.class); + FirewallRule nonVpcRule = Mockito.mock(FirewallRule.class); + + doReturn(nonVpcRule).when(_firewallMgr).createFirewallRuleForNonVPC( + Mockito.any(), Mockito.eq(caller), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.anyString(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(FirewallRuleType.class), Mockito.anyLong(), + Mockito.any(FirewallRule.TrafficType.class), Mockito.anyBoolean()); + + _firewallMgr.createFirewallRule(null, caller, "xid", 80, 80, "tcp", + Collections.singletonList("0.0.0.0/0"), null, null, null, null, + FirewallRuleType.User, 2L, null, FirewallRule.TrafficType.Ingress, true); + + verify(_firewallMgr, times(1)).createFirewallRuleForNonVPC( + Mockito.any(), Mockito.eq(caller), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.anyString(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(FirewallRuleType.class), Mockito.anyLong(), + Mockito.any(FirewallRule.TrafficType.class), Mockito.anyBoolean()); + + verify(_firewallMgr, never()).createFirewallRuleForVpc( + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + public void testApplyRulesForVpcNetworkUsesVpcProviderCheck() throws ResourceUnavailableException { + Network network = Mockito.mock(Network.class); + FirewallServiceProvider fwProvider = Mockito.mock(FirewallServiceProvider.class); + List rules = new ArrayList<>(); + FirewallRuleVO rule = new FirewallRuleVO("rule1", 1L, 80, 80, "tcp", 1L, 2, 3, Purpose.Firewall, + Collections.emptyList(), Collections.emptyList(), null, null, null, FirewallRule.TrafficType.Ingress); + rules.add(rule); + + when(network.getVpcId()).thenReturn(10L); + when(fwProvider.getProvider()).thenReturn(Network.Provider.VPCVirtualRouter); + when(_vpcMgr.isProviderSupportServiceInVpc(10L, Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(true); + when(fwProvider.applyFWRules(Mockito.eq(network), Mockito.anyList())).thenReturn(true); + _firewallMgr._firewallElements = List.of(fwProvider); + + boolean result = _firewallMgr.applyRules(network, Purpose.Firewall, rules); + + Assert.assertTrue(result); + verify(_vpcMgr, times(1)).isProviderSupportServiceInVpc(10L, Service.Firewall, Network.Provider.VPCVirtualRouter); + verify(_networkModel, never()).isProviderSupportServiceInNetwork(Mockito.anyLong(), Mockito.eq(Service.Firewall), Mockito.any()); + } + + @Test + public void testApplyRulesForNonVpcNetworkUsesNetworkModelProviderCheck() throws ResourceUnavailableException { + Network network = Mockito.mock(Network.class); + FirewallServiceProvider fwProvider = Mockito.mock(FirewallServiceProvider.class); + List rules = new ArrayList<>(); + FirewallRuleVO rule = new FirewallRuleVO("rule1", 1L, 80, 80, "tcp", 1L, 2, 3, Purpose.Firewall, + Collections.emptyList(), Collections.emptyList(), null, null, null, FirewallRule.TrafficType.Ingress); + rules.add(rule); + + when(network.getId()).thenReturn(1L); + when(network.getVpcId()).thenReturn(null); + when(fwProvider.getProvider()).thenReturn(Network.Provider.VirtualRouter); + when(_networkModel.isProviderSupportServiceInNetwork(1L, Service.Firewall, Network.Provider.VirtualRouter)).thenReturn(true); + when(fwProvider.applyFWRules(Mockito.eq(network), Mockito.anyList())).thenReturn(true); + _firewallMgr._firewallElements = List.of(fwProvider); + + boolean result = _firewallMgr.applyRules(network, Purpose.Firewall, rules); + + Assert.assertTrue(result); + verify(_networkModel, times(1)).isProviderSupportServiceInNetwork(1L, Service.Firewall, Network.Provider.VirtualRouter); + verify(_vpcMgr, never()).isProviderSupportServiceInVpc(Mockito.anyLong(), Mockito.eq(Service.Firewall), Mockito.any()); + } + + @Test + public void testGetSourceIpForIngressRuleReturnsNullWhenIdIsNull() { + IPAddressVO result = _firewallMgr.getSourceIpForIngressRule(null); + Assert.assertNull(result); + } } From 413a9f65df6473138b17af52b71b95f452c2c0a5 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Wed, 13 May 2026 14:25:04 +0530 Subject: [PATCH 19/32] Fix UnnecessaryStubbingException --- .../network/firewall/FirewallManagerTest.java | 19 ------------------- 1 file changed, 19 deletions(-) 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 5ef60c699b2f..cfe89c68117c 100644 --- a/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java +++ b/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java @@ -442,7 +442,6 @@ public void testGetFirewallServiceCapabilitiesForVpcNetworkUsesVpcProvider() { Map> providerCapabilities = new HashMap<>(); providerCapabilities.put(Service.Firewall, firewallCaps); - when(network.getId()).thenReturn(1L); when(network.getVpcId()).thenReturn(10L); when(fwProvider.getProvider()).thenReturn(Network.Provider.VPCVirtualRouter); when(fwProvider.getCapabilities()).thenReturn(providerCapabilities); @@ -591,24 +590,6 @@ public void testValidateFirewallRuleForVpcSucceedsWithSupportedProtocolAndTraffi verify(_accountMgr, times(1)).checkAccess(caller, null, true, ipAddress); } - @Test - public void testCreateIngressFirewallRuleRoutesToVpcMethodWhenIpHasVpcId() throws NetworkRuleConflictException { - FirewallRule rule = Mockito.mock(FirewallRule.class); - IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); - - when(rule.getSourceIpAddressId()).thenReturn(1L); - when(ipAddress.getVpcId()).thenReturn(10L); - - doReturn(ipAddress).when(_firewallMgr).getSourceIpForIngressRule(1L); - doReturn(rule).when(_firewallMgr).createIngressFirewallRuleForVpcIp(rule, null, ipAddress); - - doReturn(rule).when(_firewallMgr).createIngressFirewallRuleForVpcIp( - Mockito.eq(rule), Mockito.any(), Mockito.eq(ipAddress)); - - verify(_firewallMgr, never()).createIngressFirewallRuleForIsolatedIp( - Mockito.any(), Mockito.any(), Mockito.any()); - } - @Test public void testCreateFirewallRuleRoutesToVpcWhenVpcIdProvided() throws NetworkRuleConflictException { Account caller = Mockito.mock(Account.class); From e3a4f6afbd4a70b8362f497e6ef5726832c1a9a2 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Wed, 13 May 2026 15:38:49 +0530 Subject: [PATCH 20/32] Fix possible NPE --- .../cloud/network/firewall/FirewallManagerImpl.java | 7 ++++++- .../cloud/network/firewall/FirewallManagerTest.java | 10 ++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) 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 1b164eba92bb..6ea250b5e099 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -224,7 +224,12 @@ protected IPAddressVO getSourceIpForIngressRule(Long sourceIpAddressId) { if (sourceIpAddressId == null) { return null; } - return _ipAddressDao.findById(sourceIpAddressId); + IPAddressVO sourceIp = _ipAddressDao.findById(sourceIpAddressId); + if (sourceIp == null) { + throw new CloudRuntimeException("Unable to find IP address by id=" + sourceIpAddressId); + } + + return sourceIp; } protected FirewallRule createIngressFirewallRuleForIsolatedIp(FirewallRule rule, Account caller, IPAddressVO sourceIp) 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 cfe89c68117c..35657ec09025 100644 --- a/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java +++ b/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java @@ -27,6 +27,7 @@ import com.cloud.network.NetworkModel; import com.cloud.network.NetworkRuleApplier; import com.cloud.network.dao.FirewallRulesDao; +import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; @@ -43,6 +44,7 @@ import com.cloud.user.AccountManager; import com.cloud.user.DomainManager; import com.cloud.utils.component.ComponentContext; +import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; import org.apache.cloudstack.network.RoutedIpv4Manager; import org.junit.After; @@ -97,6 +99,8 @@ public class FirewallManagerTest { FirewallRulesDao _firewallDao; @Mock NetworkDao _networkDao; + @Mock + IPAddressDao _ipAddressDao; @Spy @InjectMocks @@ -694,4 +698,10 @@ public void testGetSourceIpForIngressRuleReturnsNullWhenIdIsNull() { IPAddressVO result = _firewallMgr.getSourceIpForIngressRule(null); Assert.assertNull(result); } + + @Test(expected = CloudRuntimeException.class) + public void testGetSourceIpForIngressRuleReturnsNullWhenIpIsnotPresent() { + when(_ipAddressDao.findById(1L)).thenReturn(null); + _firewallMgr.getSourceIpForIngressRule(1L); + } } From 57c1a2155b36e857991b83df6d1d9aa5d08cecce Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Wed, 13 May 2026 15:45:42 +0530 Subject: [PATCH 21/32] Fix whitespace --- test/integration/smoke/test_vpc_firewall_rules.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/smoke/test_vpc_firewall_rules.py b/test/integration/smoke/test_vpc_firewall_rules.py index 36acfaa1b345..ab9a3718415a 100644 --- a/test/integration/smoke/test_vpc_firewall_rules.py +++ b/test/integration/smoke/test_vpc_firewall_rules.py @@ -180,4 +180,3 @@ def test_02_create_firewall_rule_on_vpc_source_nat_ip(self): self.assertEqual(listed_rule.protocol.lower(), "tcp") self.assertEqual(int(listed_rule.startport), 19443) self.assertEqual(int(listed_rule.endport), 19443) - From 5b973d4c071584d277ae130d2fc8d23b37fb4fc7 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Wed, 13 May 2026 15:56:17 +0530 Subject: [PATCH 22/32] Fix vpc uuid in response --- server/src/main/java/com/cloud/api/ApiResponseHelper.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index b0f8711d5380..ab5f572021cd 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -5442,12 +5442,6 @@ public FirewallResponse createIpv6FirewallRuleResponse(FirewallRule fwRule) { } Long vpcId = fwRule.getVpcId(); - if (vpcId == null && networkId != null) { - Network network = ApiDBUtils.findNetworkById(networkId); - if (network != null) { - vpcId = network.getVpcId(); - } - } if (vpcId != null) { Vpc vpc = ApiDBUtils.findVpcById(vpcId); if (vpc != null) { From 994836a7644c10d73028d5ad78788809cf4da5f5 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Wed, 13 May 2026 18:31:56 +0530 Subject: [PATCH 23/32] Add Firewall in clone VPC offering in UI form --- ui/src/views/offering/CloneVpcOffering.vue | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/src/views/offering/CloneVpcOffering.vue b/ui/src/views/offering/CloneVpcOffering.vue index cecc0c600b57..6adbfafad611 100644 --- a/ui/src/views/offering/CloneVpcOffering.vue +++ b/ui/src/views/offering/CloneVpcOffering.vue @@ -523,6 +523,7 @@ export default { return [ { name: 'Dhcp', provider: [{ name: 'VpcVirtualRouter' }, { name: 'ConfigDrive' }] }, { name: 'Dns', provider: [{ name: 'VpcVirtualRouter' }, { name: 'ConfigDrive' }] }, + { name: 'Firewall', provider: [{ name: 'VpcVirtualRouter' }] }, { name: 'Lb', provider: [{ name: 'VpcVirtualRouter' }, { name: 'InternalLbVm' }] }, { name: 'Gateway', provider: [{ name: 'VpcVirtualRouter' }, { name: 'BigSwitchBcf' }] }, { name: 'StaticNat', provider: [{ name: 'VpcVirtualRouter' }, { name: 'BigSwitchBcf' }] }, From aa39b20cc38253b7145f0876093ef74a3013a290 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 9 Jun 2026 15:52:47 +0530 Subject: [PATCH 24/32] changes for VPC with firewall and without capabilities --- .../VirtualNetworkApplianceManagerImpl.java | 5 +++ systemvm/debian/opt/cloud/bin/configure.py | 36 ++++++++++++------- systemvm/debian/opt/cloud/bin/cs/CsAddress.py | 2 +- systemvm/debian/opt/cloud/bin/cs/CsConfig.py | 3 ++ 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index dd65719ad033..1f41cbc00af0 100644 --- a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -2168,6 +2168,11 @@ protected StringBuilder createGuestBootLoadArgs(final NicProfile guestNic, final buf.append(" ip6firewall=true"); } + if (guestNetwork.getVpcId() != null) { + boolean isVpcFirewallEnabled = vpcManager.isProviderSupportServiceInVpc(guestNetwork.getVpcId(), Service.Firewall, Provider.VPCVirtualRouter); + buf.append(" vpc_firewall_enabled=").append(isVpcFirewallEnabled); + } + final boolean isRedundant = router.getIsRedundantRouter(); if (isRedundant) { buf.append(createRedundantRouterArgs(guestNic, router)); diff --git a/systemvm/debian/opt/cloud/bin/configure.py b/systemvm/debian/opt/cloud/bin/configure.py index 5d4085b091cb..8e4a8fa0f0e3 100755 --- a/systemvm/debian/opt/cloud/bin/configure.py +++ b/systemvm/debian/opt/cloud/bin/configure.py @@ -703,30 +703,28 @@ def process(self): self.add_routing_rules() return - desired_firewall_ips = self._get_desired_vpc_firewall_ips() + desired_firewall_ips = set() + if self.config.is_vpc() and self.config.is_vpc_firewall_enabled(): + desired_firewall_ips = self._get_desired_vpc_firewall_ips() + fw_chains_created = set() + for item in self.dbag: if item == "id": continue if self.config.is_vpc() and not ("purpose" in self.dbag[item] and self.dbag[item]["purpose"] == "Firewall"): self.AclDevice(self.dbag[item], self.config).create() else: + if self.config.is_vpc() and self.dbag[item].get("purpose") == "Firewall" and not self.config.is_vpc_firewall_enabled(): + continue # For VPC firewall rules, create the PREROUTING jump and chain skeleton # once per public IP before adding the individual rule - if self.config.is_vpc() and self.dbag[item].get("purpose") == "Firewall": + if self.config.is_vpc() and self.config.is_vpc_firewall_enabled() and self.dbag[item].get("purpose") == "Firewall": src_ip = self.dbag[item].get("src_ip") - if src_ip and src_ip not in fw_chains_created: - fw = self.config.get_fw() - fw.append(["mangle", "front", - "-A PREROUTING -d %s/32 -j FIREWALL_%s" % (src_ip, src_ip)]) - fw.append(["mangle", "front", - "-A FIREWALL_%s -m state --state RELATED,ESTABLISHED -j RETURN" % src_ip]) - fw.append(["mangle", "", - "-A FIREWALL_%s -j DROP" % src_ip]) - fw_chains_created.add(src_ip) + self._ensure_vpc_firewall_chains([src_ip], fw_chains_created) self.AclIP(self.dbag[item], self.config).create() - if self.config.is_vpc(): + if self.config.is_vpc() and self.config.is_vpc_firewall_enabled(): self._cleanup_removed_vpc_firewall_chains(desired_firewall_ips) def _get_desired_vpc_firewall_ips(self): @@ -744,8 +742,20 @@ def _get_desired_vpc_firewall_ips(self): desired_firewall_ips.add(src_ip) return desired_firewall_ips + def _ensure_vpc_firewall_chains(self, source_ips, fw_chains_created): + fw = self.config.get_fw() + for src_ip in source_ips: + if not src_ip or src_ip in fw_chains_created: + continue + fw.append(["mangle", "front", + "-A PREROUTING -d %s/32 -j FIREWALL_%s" % (src_ip, src_ip)]) + fw.append(["mangle", "front", + "-A FIREWALL_%s -m state --state RELATED,ESTABLISHED -j RETURN" % src_ip]) + fw.append(["mangle", "", + "-A FIREWALL_%s -j DROP" % src_ip]) + fw_chains_created.add(src_ip) + def _cleanup_removed_vpc_firewall_chains(self, desired_firewall_ips): - """Delete FIREWALL_ chain only when no firewall rule remains for that VPC public IP.""" try: mangle_save = CsHelper.execute("iptables-save -t mangle") existing_firewall_ips = [] diff --git a/systemvm/debian/opt/cloud/bin/cs/CsAddress.py b/systemvm/debian/opt/cloud/bin/cs/CsAddress.py index 0ef5d45402da..fe95808f7d32 100755 --- a/systemvm/debian/opt/cloud/bin/cs/CsAddress.py +++ b/systemvm/debian/opt/cloud/bin/cs/CsAddress.py @@ -647,7 +647,6 @@ def fw_vpcrouter(self): (self.address['network'], self.address['network'])]) if self.get_type() in ["public"]: - self.fw.append( ["mangle", "", "-A FORWARD -j VPN_STATS_%s" % self.dev]) self.fw.append( @@ -681,6 +680,7 @@ def fw_vpcrouter(self): self.fw.append(["filter", "", "-P INPUT DROP"]) self.fw.append(["filter", "", "-P FORWARD DROP"]) + def fw_router_routing(self): if self.config.is_vpc() or not self.config.is_routed(): return diff --git a/systemvm/debian/opt/cloud/bin/cs/CsConfig.py b/systemvm/debian/opt/cloud/bin/cs/CsConfig.py index 549b08f75fc6..5218e8111161 100755 --- a/systemvm/debian/opt/cloud/bin/cs/CsConfig.py +++ b/systemvm/debian/opt/cloud/bin/cs/CsConfig.py @@ -155,3 +155,6 @@ def get_egress_table(self): def has_public_network(self): return self.cmdline().idata().get('has_public_network', 'true') == 'true' + + def is_vpc_firewall_enabled(self): + return self.cmdline().idata().get('vpc_firewall_enabled', 'false') == 'true' From 6f466217aacd37b64eb40705529ec3fe22522eb2 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 9 Jun 2026 16:25:01 +0530 Subject: [PATCH 25/32] Add VPC firewall check --- .../cloud/network/firewall/FirewallManagerImpl.java | 3 +++ .../cloud/network/firewall/FirewallManagerTest.java | 11 +++++++++++ 2 files changed, 14 insertions(+) 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 6ea250b5e099..9a0771c41b45 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -439,6 +439,9 @@ protected void validateFirewallRuleForVpc(Account caller, IPAddressVO ipAddress, Map caps = null; if (purpose == Purpose.Firewall) { caps = getFirewallServiceCapabilitiesForVpc(vpcId); + if (caps == null) { + throw new InvalidParameterValueException("Firewall service is not supported in VPC " + vpc); + } } if (caps != null) { 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 35657ec09025..e3e949ac1d02 100644 --- a/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java +++ b/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java @@ -549,6 +549,17 @@ public void testValidateFirewallRuleForVpcThrowsWhenActiveVpcNotFound() { _firewallMgr.validateFirewallRuleForVpc(caller, ipAddress, 80, 80, "tcp", Purpose.Firewall, FirewallRuleType.User, 10L, FirewallRule.TrafficType.Ingress); } + @Test(expected = InvalidParameterValueException.class) + public void testValidateFirewallRuleForVpcThrowsWhenFirewallServiceNotSupported() { + Account caller = Mockito.mock(Account.class); + IPAddressVO ipAddress = Mockito.mock(IPAddressVO.class); + Vpc vpc = Mockito.mock(Vpc.class); + when(_vpcMgr.getActiveVpc(10L)).thenReturn(vpc); + _firewallMgr._firewallElements = Collections.emptyList(); + + _firewallMgr.validateFirewallRuleForVpc(caller, ipAddress, 80, 80, "tcp", Purpose.Firewall, FirewallRuleType.User, 10L, FirewallRule.TrafficType.Ingress); + } + @Test(expected = InvalidParameterValueException.class) public void testValidateFirewallRuleForVpcThrowsOnUnsupportedProtocol() { Account caller = Mockito.mock(Account.class); From f75771b025bb23de9d08b9ad30a4edfa64f3ce0e Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Thu, 11 Jun 2026 11:07:00 +0530 Subject: [PATCH 26/32] UI fix for VPC no firewall --- ui/src/views/network/PublicIpResource.vue | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/ui/src/views/network/PublicIpResource.vue b/ui/src/views/network/PublicIpResource.vue index 0f77cb52937a..15c423071575 100644 --- a/ui/src/views/network/PublicIpResource.vue +++ b/ui/src/views/network/PublicIpResource.vue @@ -145,16 +145,21 @@ export default { return } - // VPC IPs with static nat keep existing VPN behavior and always show firewall + // VPC IPs with static nat keep existing VPN behavior; show firewall only when capability exists if (this.resource.isstaticnat) { - const tabs = this.addFirewallTab(this.$route.meta.tabs).map(tab => { - if (tab.name !== 'firewall') { - return tab - } - const staticNatFirewallTab = { ...tab } - delete staticNatFirewallTab.networkServiceFilter - return staticNatFirewallTab - }) + let tabs = this.$route.meta.tabs + if (hasFirewallCapability) { + tabs = this.addFirewallTab(tabs).map(tab => { + if (tab.name !== 'firewall') { + return tab + } + const staticNatFirewallTab = { ...tab } + delete staticNatFirewallTab.networkServiceFilter + return staticNatFirewallTab + }) + } else { + tabs = tabs.filter(tab => tab.name !== 'firewall') + } this.tabs = tabs return } From 1497807f3c79df0d3aef874b57025b1096969f92 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Thu, 11 Jun 2026 19:29:15 +0530 Subject: [PATCH 27/32] Make the drop rule add by default --- .../VirtualNetworkApplianceManagerImpl.java | 7 +- systemvm/debian/opt/cloud/bin/configure.py | 66 ++++++++++++++++--- 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index 1f41cbc00af0..90a26bdb80eb 100644 --- a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -2018,6 +2018,8 @@ public boolean finalizeVirtualMachineProfile(final VirtualMachineProfile profile } else { buf.append(" has_public_network=false"); } + boolean isVpcFirewallEnabled = vpcManager.isProviderSupportServiceInVpc(vpc.getId(), Service.Firewall, Provider.VPCVirtualRouter); + buf.append(" vpc_firewall_enabled=").append(isVpcFirewallEnabled); } else if (!publicNetwork) { type = "dhcpsrvr"; } else { @@ -2168,11 +2170,6 @@ protected StringBuilder createGuestBootLoadArgs(final NicProfile guestNic, final buf.append(" ip6firewall=true"); } - if (guestNetwork.getVpcId() != null) { - boolean isVpcFirewallEnabled = vpcManager.isProviderSupportServiceInVpc(guestNetwork.getVpcId(), Service.Firewall, Provider.VPCVirtualRouter); - buf.append(" vpc_firewall_enabled=").append(isVpcFirewallEnabled); - } - final boolean isRedundant = router.getIsRedundantRouter(); if (isRedundant) { buf.append(createRedundantRouterArgs(guestNic, router)); diff --git a/systemvm/debian/opt/cloud/bin/configure.py b/systemvm/debian/opt/cloud/bin/configure.py index 8e4a8fa0f0e3..77b56779d5b8 100755 --- a/systemvm/debian/opt/cloud/bin/configure.py +++ b/systemvm/debian/opt/cloud/bin/configure.py @@ -704,10 +704,13 @@ def process(self): return desired_firewall_ips = set() + fw_chains_created = set() if self.config.is_vpc() and self.config.is_vpc_firewall_enabled(): desired_firewall_ips = self._get_desired_vpc_firewall_ips() - - fw_chains_created = set() + # Pre-create FIREWALL chains for ALL public IPs that have any active rule + # (static NAT, port forwarding, LB, or explicit firewall rule) so that the + # default DROP is always in place even before any explicit firewall rule exists. + self._ensure_vpc_firewall_chains(desired_firewall_ips, fw_chains_created) for item in self.dbag: if item == "id": @@ -717,8 +720,8 @@ def process(self): else: if self.config.is_vpc() and self.dbag[item].get("purpose") == "Firewall" and not self.config.is_vpc_firewall_enabled(): continue - # For VPC firewall rules, create the PREROUTING jump and chain skeleton - # once per public IP before adding the individual rule + # Chain skeleton is already ensured by the pre-creation pass above; + # _ensure_vpc_firewall_chains is idempotent (skips IPs in fw_chains_created). if self.config.is_vpc() and self.config.is_vpc_firewall_enabled() and self.dbag[item].get("purpose") == "Firewall": src_ip = self.dbag[item].get("src_ip") self._ensure_vpc_firewall_chains([src_ip], fw_chains_created) @@ -728,10 +731,23 @@ def process(self): self._cleanup_removed_vpc_firewall_chains(desired_firewall_ips) def _get_desired_vpc_firewall_ips(self): - desired_firewall_ips = set() + """ + Collect the full set of public IPs that should have a FIREWALL mangle chain + in a VPC with firewall capability. This includes IPs from explicit firewall + rules, forwarding/static-NAT rules, and load-balancer rules. + """ if not self.config.is_vpc(): - return desired_firewall_ips + return set() + + ips = set() + ips.update(self._get_firewall_rule_ips()) + ips.update(self._get_forwarding_rule_ips()) + ips.update(self._get_loadbalancer_ips()) + return ips + def _get_firewall_rule_ips(self): + """Return public IPs that have explicit firewall rules in this data bag.""" + ips = set() for item in self.dbag: if item == "id": continue @@ -739,8 +755,42 @@ def _get_desired_vpc_firewall_ips(self): if rule.get("purpose") == "Firewall": src_ip = rule.get("src_ip") if src_ip: - desired_firewall_ips.add(src_ip) - return desired_firewall_ips + ips.add(src_ip) + return ips + + def _get_forwarding_rule_ips(self): + """ + Return public IPs from the forwardingrules bag (static NAT and port forwarding). + That bag is keyed by public IP, so each key (other than 'id') is a public IP. + """ + ips = set() + try: + fwd_bag = CsDataBag("forwardingrules", self.config) + for public_ip in fwd_bag.get_bag(): + if public_ip == "id": + continue + ips.add(public_ip) + except Exception as e: + logging.debug("Could not load forwardingrules for VPC firewall chain collection: %s", e) + return ips + + def _get_loadbalancer_ips(self): + """ + Return public IPs from the loadbalancer bag. + add_rules entries are formatted as 'ip:port', so the IP is the first segment. + """ + ips = set() + try: + lb_bag = CsDataBag("loadbalancer", self.config) + lb_data = lb_bag.get_bag() + if "config" in lb_data and lb_data["config"]: + for rule_str in lb_data["config"][0].get("add_rules", []): + ip = rule_str.split(":")[0] + if ip: + ips.add(ip) + except Exception as e: + logging.debug("Could not load loadbalancer for VPC firewall chain collection: %s", e) + return ips def _ensure_vpc_firewall_chains(self, source_ips, fw_chains_created): fw = self.config.get_fw() From f0171f9d1239f30d7a398f45a95edeb0ca8525bf Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 16 Jun 2026 11:23:51 +0530 Subject: [PATCH 28/32] Fix restart VPC with cleanup --- .../cloud/network/dao/FirewallRulesDao.java | 2 ++ .../network/dao/FirewallRulesDaoImpl.java | 17 +++++++++++++ .../network/router/CommandSetupHelper.java | 6 +++-- ...VpcVirtualNetworkApplianceManagerImpl.java | 25 +++++++++++++++++++ 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDao.java b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDao.java index 7f322ae6c037..3527ce84dcf6 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDao.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDao.java @@ -69,6 +69,8 @@ public interface FirewallRulesDao extends GenericDao { List listByNetworkPurposeTrafficType(long networkId, FirewallRule.Purpose purpose, FirewallRule.TrafficType trafficType); + List listByVpcPurposeTrafficType(long vpcId, FirewallRule.Purpose purpose, FirewallRule.TrafficType trafficType); + List listByIpAndPurposeWithState(Long addressId, FirewallRule.Purpose purpose, FirewallRule.State state); void loadSourceCidrs(FirewallRuleVO rule); diff --git a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java index 57d53f92572c..5a1e1aae6b66 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java @@ -74,6 +74,7 @@ protected FirewallRulesDaoImpl() { AllFieldsSearch.and("domain", AllFieldsSearch.entity().getDomainId(), Op.EQ); AllFieldsSearch.and("id", AllFieldsSearch.entity().getId(), Op.EQ); AllFieldsSearch.and("networkId", AllFieldsSearch.entity().getNetworkId(), Op.EQ); + AllFieldsSearch.and("vpcId", AllFieldsSearch.entity().getVpcId(), Op.EQ); AllFieldsSearch.and("related", AllFieldsSearch.entity().getRelated(), Op.EQ); AllFieldsSearch.and("trafficType", AllFieldsSearch.entity().getTrafficType(), Op.EQ); AllFieldsSearch.done(); @@ -356,6 +357,22 @@ public List listByNetworkPurposeTrafficType(long networkId, Purp return listBy(sc); } + @Override + public List listByVpcPurposeTrafficType(long vpcId, Purpose purpose, TrafficType trafficType) { + SearchCriteria sc = AllFieldsSearch.create(); + sc.setParameters("vpcId", vpcId); + + if (purpose != null) { + sc.setParameters("purpose", purpose); + } + + if (trafficType != null) { + sc.setParameters("trafficType", trafficType); + } + + return listBy(sc); + } + @Override @DB public boolean remove(Long id) { diff --git a/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java b/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java index 628c36826dec..9c36c6b924a3 100644 --- a/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java +++ b/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java @@ -551,7 +551,7 @@ public void createApplyIpv6FirewallRulesCommands(final List rules, final VirtualRouter router, final Commands cmds, final long guestNetworkId) { + public void createFirewallRulesCommands(final List rules, final VirtualRouter router, final Commands cmds, final Long guestNetworkId) { final List rulesTO = new ArrayList<>(); String systemRule = null; Boolean defaultEgressPolicy = false; @@ -585,7 +585,9 @@ public void createFirewallRulesCommands(final List rules final SetFirewallRulesCommand cmd = new SetFirewallRulesCommand(rulesTO); cmd.setAccessDetail(NetworkElementCommand.ROUTER_IP, _routerControlHelper.getRouterControlIp(router.getId())); - cmd.setAccessDetail(NetworkElementCommand.ROUTER_GUEST_IP, _routerControlHelper.getRouterIpInNetwork(guestNetworkId, router.getId())); + if (guestNetworkId != null) { + cmd.setAccessDetail(NetworkElementCommand.ROUTER_GUEST_IP, _routerControlHelper.getRouterIpInNetwork(guestNetworkId, router.getId())); + } cmd.setAccessDetail(NetworkElementCommand.ROUTER_NAME, router.getInstanceName()); final DataCenterVO dcVo = _dcDao.findById(router.getDataCenterId()); cmd.setAccessDetail(NetworkElementCommand.ZONE_NETWORK_TYPE, dcVo.getNetworkType().toString()); diff --git a/server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java b/server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java index 67cfe8458518..aa772d01fb79 100644 --- a/server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java +++ b/server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java @@ -72,6 +72,7 @@ import com.cloud.network.dao.LoadBalancerVO; import com.cloud.network.dao.MonitoringServiceVO; import com.cloud.network.dao.NetworkVO; +import com.cloud.network.rules.FirewallRule; import com.cloud.network.dao.RemoteAccessVpnVO; import com.cloud.network.dao.Site2SiteVpnConnectionVO; import com.cloud.network.lb.LoadBalancingRule; @@ -567,6 +568,10 @@ public boolean finalizeCommandsOnStart(final Commands cmds, final VirtualMachine finalizeMonitorService(cmds, profile, domainRouterVO, provider, publicNics.get(0).second().getId(), true, routerHealthCheckConfig); } + if (reprogramGuestNtwks) { + reapplyVpcFirewallIngressRules(cmds, domainRouterVO, provider); + } + for (final Pair nicNtwk : guestNics) { final Nic guestNic = nicNtwk.first(); final long guestNetworkId = guestNic.getNetworkId(); @@ -638,6 +643,26 @@ protected void finalizeNetworkRulesForNetwork(final Commands cmds, final DomainR } } + private void reapplyVpcFirewallIngressRules(final Commands cmds, final DomainRouterVO domainRouterVO, final Provider provider) { + final Long vpcId = domainRouterVO.getVpcId(); + if (vpcId == null) { + return; + } + + if (!_vpcMgr.isProviderSupportServiceInVpc(vpcId, Service.Firewall, provider)) { + return; + } + + final List firewallRulesIngress = new ArrayList<>( + _rulesDao.listByVpcPurposeTrafficType(vpcId, FirewallRule.Purpose.Firewall, FirewallRule.TrafficType.Ingress)); + if (firewallRulesIngress.isEmpty()) { + return; + } + + logger.debug("Found {} VPC firewall ingress rule(s) to apply as a part of domR {} start for VPC {}", firewallRulesIngress.size(), domainRouterVO, vpcId); + _commandSetupHelper.createFirewallRulesCommands(firewallRulesIngress, domainRouterVO, cmds, null); + } + protected boolean sendNetworkRulesToRouter(final long routerId, final long networkId, final boolean reprogramNetwork) throws ResourceUnavailableException { final DomainRouterVO router = _routerDao.findById(routerId); final Commands cmds = new Commands(OnError.Continue); From 3dadaa5ef8f8ca514d3ee63f9d497ef388391388 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 16 Jun 2026 11:57:55 +0530 Subject: [PATCH 29/32] Fix cleanup order --- .../smoke/test_vpc_firewall_rules.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/test/integration/smoke/test_vpc_firewall_rules.py b/test/integration/smoke/test_vpc_firewall_rules.py index ab9a3718415a..6aae5f13fbcd 100644 --- a/test/integration/smoke/test_vpc_firewall_rules.py +++ b/test/integration/smoke/test_vpc_firewall_rules.py @@ -43,7 +43,10 @@ def setUpClass(cls): ) cls._cleanup.append(cls.account) - cls.services["vpc_offering"]["supportedservices"] = "Vpn,Dhcp,Dns,SourceNat,Lb,UserData,StaticNat,NetworkACL,PortForwarding,Firewall" + cls.services["vpc_offering"]["supportedservices"] = ( + "Vpn,Dhcp,Dns,SourceNat,Lb,UserData,StaticNat," + "NetworkACL,PortForwarding,Firewall" + ) cls.services["vpc_offering"]["serviceProviderList"] = { "Vpn": "VpcVirtualRouter", "Dhcp": "VpcVirtualRouter", @@ -68,10 +71,11 @@ def setUpClass(cls): cls.apiclient, name="DefaultIsolatedNetworkOfferingForVpcNetworks" ) - cls.assertTrue(network_offering is not None and len(network_offering) > 0, - "No VPC tier network offering found") + cls.assertTrue( + network_offering is not None and len(network_offering) > 0, + "No VPC tier network offering found" + ) cls.network_offering = network_offering[0] - cls.services["vpc"]["cidr"] = "10.20.30.0/24" cls.vpc = VPC.create( cls.apiclient, @@ -81,11 +85,13 @@ def setUpClass(cls): account=cls.account.name, domainid=cls.account.domainid ) - cls._cleanup.append(cls.vpc) cls.tier = Network.create( cls.apiclient, - services={"name": "vpc-fw-tier", "displaytext": "vpc-fw-tier"}, + services={ + "name": "vpc-fw-tier", + "displaytext": "vpc-fw-tier" + }, accountid=cls.account.name, domainid=cls.account.domainid, networkofferingid=cls.network_offering.id, @@ -94,7 +100,6 @@ def setUpClass(cls): gateway="10.20.30.1", netmask="255.255.255.0" ) - cls._cleanup.append(cls.tier) @classmethod def tearDownClass(cls): From d029be6c92492ff0eb3877ec4f8b261adca4634f Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Fri, 10 Jul 2026 09:44:59 +0530 Subject: [PATCH 30/32] Fix FW rule application on a VPC not having tiers --- .../com/cloud/network/NetworkRuleApplier.java | 7 +- .../element/FirewallServiceProvider.java | 18 ++- .../api/routing/NetworkElementCommand.java | 1 + .../api/routing/SetFirewallRulesCommand.java | 10 ++ .../cloud/network/IpAddressManagerImpl.java | 14 +-- .../network/element/VirtualRouterElement.java | 7 +- .../element/VpcVirtualRouterElement.java | 52 ++++++-- .../network/firewall/FirewallManagerImpl.java | 23 +++- .../network/router/CommandSetupHelper.java | 36 +++++- .../VirtualNetworkApplianceManagerImpl.java | 4 +- ...VpcVirtualNetworkApplianceManagerImpl.java | 2 +- .../cloud/network/rules/FirewallRules.java | 11 ++ .../com/cloud/network/rules/RuleApplier.java | 12 ++ .../topology/BasicNetworkTopology.java | 117 ++++++++++++++++++ .../network/topology/BasicNetworkVisitor.java | 4 +- .../network/topology/NetworkTopology.java | 6 + .../cloud/network/IpAddressManagerTest.java | 67 +++------- .../element/VpcVirtualRouterElementTest.java | 7 +- .../network/firewall/FirewallManagerTest.java | 16 ++- 19 files changed, 324 insertions(+), 90 deletions(-) diff --git a/api/src/main/java/com/cloud/network/NetworkRuleApplier.java b/api/src/main/java/com/cloud/network/NetworkRuleApplier.java index b9942e71eb26..69b712bc6ca2 100644 --- a/api/src/main/java/com/cloud/network/NetworkRuleApplier.java +++ b/api/src/main/java/com/cloud/network/NetworkRuleApplier.java @@ -21,8 +21,13 @@ import com.cloud.exception.ResourceUnavailableException; import com.cloud.network.rules.FirewallRule; +import com.cloud.network.vpc.Vpc; public interface NetworkRuleApplier { - public boolean applyRules(Network network, FirewallRule.Purpose purpose, List rules) throws ResourceUnavailableException; + default boolean applyRules(Network network, FirewallRule.Purpose purpose, List rules) throws ResourceUnavailableException { + return applyRules(network, null, purpose, rules); + } + + boolean applyRules(Network network, Vpc vpc, FirewallRule.Purpose purpose, List rules) throws ResourceUnavailableException; } diff --git a/api/src/main/java/com/cloud/network/element/FirewallServiceProvider.java b/api/src/main/java/com/cloud/network/element/FirewallServiceProvider.java index c091142d9353..6b0f932e8c22 100644 --- a/api/src/main/java/com/cloud/network/element/FirewallServiceProvider.java +++ b/api/src/main/java/com/cloud/network/element/FirewallServiceProvider.java @@ -21,14 +21,20 @@ import com.cloud.exception.ResourceUnavailableException; import com.cloud.network.Network; import com.cloud.network.rules.FirewallRule; +import com.cloud.network.vpc.Vpc; public interface FirewallServiceProvider extends NetworkElement { /** - * Apply rules - * @param network - * @param rules - * @return - * @throws ResourceUnavailableException + * Apply firewall rules in a network context. */ - boolean applyFWRules(Network network, List rules) throws ResourceUnavailableException; + default boolean applyFWRules(Network network, List rules) throws ResourceUnavailableException { + return false; + } + + /** + * Apply firewall rules in a VPC context. + */ + default boolean applyFWRulesInVPC(Vpc vpc, List rules) throws ResourceUnavailableException { + return false; + } } diff --git a/core/src/main/java/com/cloud/agent/api/routing/NetworkElementCommand.java b/core/src/main/java/com/cloud/agent/api/routing/NetworkElementCommand.java index 400b6bb80917..20d2ea0a443d 100644 --- a/core/src/main/java/com/cloud/agent/api/routing/NetworkElementCommand.java +++ b/core/src/main/java/com/cloud/agent/api/routing/NetworkElementCommand.java @@ -37,6 +37,7 @@ public abstract class NetworkElementCommand extends Command { public static final String ZONE_NETWORK_TYPE = "zone.network.type"; public static final String GUEST_BRIDGE = "guest.bridge"; public static final String VPC_PRIVATE_GATEWAY = "vpc.gateway.private"; + public static final String VPC_ID = "vpc.id"; public static final String FIREWALL_EGRESS_DEFAULT = "firewall.egress.default"; public static final String NETWORK_PUB_LAST_IP = "network.public.last.ip"; public static final String HYPERVISOR_HOST_PRIVATE_IP = "hypervisor.private.ip"; diff --git a/core/src/main/java/com/cloud/agent/api/routing/SetFirewallRulesCommand.java b/core/src/main/java/com/cloud/agent/api/routing/SetFirewallRulesCommand.java index c56f8d20fbe6..ff81ab7749c5 100644 --- a/core/src/main/java/com/cloud/agent/api/routing/SetFirewallRulesCommand.java +++ b/core/src/main/java/com/cloud/agent/api/routing/SetFirewallRulesCommand.java @@ -32,6 +32,7 @@ */ public class SetFirewallRulesCommand extends NetworkElementCommand { FirewallRuleTO[] rules; + Long vpcId; protected SetFirewallRulesCommand() { } @@ -40,10 +41,19 @@ public SetFirewallRulesCommand(List rules) { this.rules = rules.toArray(new FirewallRuleTO[rules.size()]); } + public SetFirewallRulesCommand(List rules, Long vpcId) { + this.rules = rules.toArray(new FirewallRuleTO[rules.size()]); + this.vpcId = vpcId; + } + public FirewallRuleTO[] getRules() { return rules; } + public Long getVpcId() { + return vpcId; + } + public String[][] generateFwRules() { String[][] result = new String[2][]; Set toAdd = new HashSet(); diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index b7e147414e74..3cd42c50c28d 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -666,14 +666,12 @@ public boolean applyRules(List rules, FirewallRule.Purpo List publicIps = new ArrayList(); // For VPC firewall rules the networkId on the rule is null; resolve via VPC. - Network network; + Network network = null; + Vpc vpc = null; if (networkId != null) { network = _networksDao.findById(networkId); } else if (vpcId != null) { - List vpcNetworks = _vpcMgr.getVpcNetworks(vpcId); - network = (vpcNetworks != null && !vpcNetworks.isEmpty()) ? _networksDao.findById(vpcNetworks.get(0).getId()) : null; - } else { - network = null; + vpc = _vpcDao.findById(vpcId); } if (network == null) { @@ -707,10 +705,10 @@ public boolean applyRules(List rules, FirewallRule.Purpo } try { - if (network != null) { - applier.applyRules(network, purpose, rules); + if (network != null || vpc != null) { + applier.applyRules(network, vpc, purpose, rules); } else { - logger.warn("Skipping applyRules: no network resolved for rules (vpcId={})", vpcId); + logger.warn("Skipping applyRules: no network or vpc resolved for rules (networkId={}, vpcId={})", networkId, vpcId); success = false; } } catch (ResourceUnavailableException e) { diff --git a/server/src/main/java/com/cloud/network/element/VirtualRouterElement.java b/server/src/main/java/com/cloud/network/element/VirtualRouterElement.java index 0978fcba51b5..1845574de861 100644 --- a/server/src/main/java/com/cloud/network/element/VirtualRouterElement.java +++ b/server/src/main/java/com/cloud/network/element/VirtualRouterElement.java @@ -212,6 +212,10 @@ protected boolean canHandle(final Network network, final Service service) { return true; } + protected boolean canHandle(final Vpc vpc, final Service service) { + return false; + } + @Override public boolean implement(final Network network, final NetworkOffering offering, final DeployDestination dest, final ReservationContext context) throws ResourceUnavailableException, ConcurrentOperationException, InsufficientCapacityException { @@ -281,7 +285,7 @@ public boolean applyFWRules(final Network network, final List routers = getRouters(network); if (routers == null || routers.isEmpty()) { - logger.debug("Virtual router element doesn't need to apply firewall rules on the backend; virtual router doesn't exist in the network {}", network); + logger.debug("Virtual router element doesn't need to apply firewall rules on the backend; virtual router doesn't exist in the network {}"); return true; } @@ -304,6 +308,7 @@ public boolean applyFWRules(final Network network, final List rules = new ArrayList(); diff --git a/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java b/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java index c1311c96eb73..99f8d8f697b2 100644 --- a/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java +++ b/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java @@ -52,6 +52,7 @@ import com.cloud.network.router.VirtualRouter.Role; import com.cloud.network.router.VpcNetworkHelperImpl; import com.cloud.network.router.VpcVirtualNetworkApplianceManager; +import com.cloud.network.rules.FirewallRule; import com.cloud.network.vpc.NetworkACLItem; import com.cloud.network.vpc.NetworkACLItemDao; import com.cloud.network.vpc.NetworkACLItemVO; @@ -139,13 +140,7 @@ protected boolean canHandle(final Network network, final Service service) { return false; } } else { - boolean supportsService; - if (service == Service.Firewall) { - supportsService = _vpcMgr.isProviderSupportServiceInVpc(network.getVpcId(), service, getProvider()); - } else { - supportsService = _networkMdl.isProviderSupportServiceInNetwork(network.getId(), service, getProvider()); - } - if (!supportsService) { + if (!_networkMdl.isProviderSupportServiceInNetwork(network.getId(), service, getProvider())) { logger.trace("Element " + getProvider().getName() + " doesn't support service " + service.getName() + " in the network " + network); return false; } @@ -154,6 +149,49 @@ protected boolean canHandle(final Network network, final Service service) { return true; } + @Override + protected boolean canHandle(final Vpc vpc, final Service service) { + if (vpc == null) { + return false; + } + + if (!_networkMdl.isProviderEnabledInZone(vpc.getZoneId(), Network.Provider.VPCVirtualRouter.getName())) { + return false; + } + + if (service != null && !_vpcMgr.isProviderSupportServiceInVpc(vpc.getId(), service, getProvider())) { + logger.trace("Element " + getProvider().getName() + " doesn't support service " + service.getName() + " in the vpc " + vpc); + return false; + } + + return true; + } + + @Override + public boolean applyFWRulesInVPC(final Vpc vpc, final List rules) throws ResourceUnavailableException { + boolean result = true; + if (canHandle(vpc, Service.Firewall)) { + final List routers = _routerDao.listByVpcId(vpc.getId()); + if (CollectionUtils.isEmpty(routers)) { + logger.debug("Virtual router element doesn't need to apply firewall rules on the backend; virtual router doesn't exist in the vpc"); + return true; + } + + Network network = null; + if (CollectionUtils.isNotEmpty(rules) && rules.get(0).getNetworkId() != null) { + network = _networkModel.getNetwork(rules.get(0).getNetworkId()); + } + + final DataCenterVO dcVO = _dcDao.findById(vpc.getZoneId()); + final NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO); + + for (final DomainRouterVO domainRouterVO : routers) { + result = result && networkTopology.applyFirewallRulesInVPC(vpc, rules, domainRouterVO); + } + } + return result; + } + @Override public boolean implementVpc(final Vpc vpc, final DeployDestination dest, final ReservationContext context) throws ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException { 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 9a0771c41b45..dc6fc12a6110 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -938,7 +938,7 @@ public boolean applyRules(List rules, boolean continueOn } @Override - public boolean applyRules(Network network, Purpose purpose, List rules) throws ResourceUnavailableException { + public boolean applyRules(Network network, Vpc vpc, Purpose purpose, List rules) throws ResourceUnavailableException { boolean handled = false; switch (purpose) { /* StaticNatRule would be applied by Firewall provider, since the incompatible of two object */ @@ -948,15 +948,25 @@ public boolean applyRules(Network network, Purpose purpose, List rules) throws ResourceUnavailableException { + return applyRules(network, null, purpose, rules); + } + @Override public void removeRule(FirewallRule rule) { diff --git a/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java b/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java index 9c36c6b924a3..58b2892e09c3 100644 --- a/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java +++ b/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java @@ -463,7 +463,8 @@ public void createApplyStaticNatRulesCommands(final List rules, final VirtualRouter router, final Commands cmds, final long guestNetworkId) { + public void createApplyFirewallRulesCommands(final List rules, final VirtualRouter router, final Commands cmds, + final Long guestNetworkId, final Long vpcId) { final List rulesTO = new ArrayList<>(); String systemRule = null; Boolean defaultEgressPolicy = false; @@ -485,6 +486,10 @@ public void createApplyFirewallRulesCommands(final List final FirewallRuleTO ruleTO = new FirewallRuleTO(rule, null, srcIp, Purpose.Firewall, traffictype); rulesTO.add(ruleTO); } else if (rule.getTrafficType() == FirewallRule.TrafficType.Egress) { + if (guestNetworkId == null) { + logger.warn("Skipping egress firewall rule {} as guestNetworkId is null", rule.getUuid()); + continue; + } final NetworkVO network = _networkDao.findById(guestNetworkId); final NetworkOfferingVO offering = _networkOfferingDao.findById(network.getNetworkOfferingId()); defaultEgressPolicy = offering.isEgressDefaultPolicy(); @@ -495,9 +500,14 @@ public void createApplyFirewallRulesCommands(final List } } - final SetFirewallRulesCommand cmd = new SetFirewallRulesCommand(rulesTO); + final SetFirewallRulesCommand cmd = new SetFirewallRulesCommand(rulesTO, vpcId); cmd.setAccessDetail(NetworkElementCommand.ROUTER_IP, _routerControlHelper.getRouterControlIp(router.getId())); - cmd.setAccessDetail(NetworkElementCommand.ROUTER_GUEST_IP, _routerControlHelper.getRouterIpInNetwork(guestNetworkId, router.getId())); + if (guestNetworkId != null) { + cmd.setAccessDetail(NetworkElementCommand.ROUTER_GUEST_IP, _routerControlHelper.getRouterIpInNetwork(guestNetworkId, router.getId())); + } + if (vpcId != null) { + cmd.setAccessDetail(NetworkElementCommand.VPC_ID, String.valueOf(vpcId)); + } cmd.setAccessDetail(NetworkElementCommand.ROUTER_NAME, router.getInstanceName()); final DataCenterVO dcVo = _dcDao.findById(router.getDataCenterId()); cmd.setAccessDetail(NetworkElementCommand.ZONE_NETWORK_TYPE, dcVo.getNetworkType().toString()); @@ -510,6 +520,10 @@ public void createApplyFirewallRulesCommands(final List cmds.addCommand(cmd); } + public void createApplyFirewallRulesCommands(final List rules, final VirtualRouter router, final Commands cmds, final long guestNetworkId) { + createApplyFirewallRulesCommands(rules, router, cmds, guestNetworkId, null); + } + public void createApplyIpv6FirewallRulesCommands(final List rules, final VirtualRouter router, final Commands cmds, final long guestNetworkId) { final List rulesTO = new ArrayList<>(); String systemRule = null; @@ -551,7 +565,8 @@ public void createApplyIpv6FirewallRulesCommands(final List rules, final VirtualRouter router, final Commands cmds, final Long guestNetworkId) { + public void createFirewallRulesCommands(final List rules, final VirtualRouter router, final Commands cmds, final Long guestNetworkId, + final Long vpcId) { final List rulesTO = new ArrayList<>(); String systemRule = null; Boolean defaultEgressPolicy = false; @@ -573,6 +588,10 @@ public void createFirewallRulesCommands(final List rules final FirewallRuleTO ruleTO = new FirewallRuleTO(rule, null, srcIp, Purpose.Firewall, traffictype); rulesTO.add(ruleTO); } else if (rule.getTrafficType() == FirewallRule.TrafficType.Egress) { + if (guestNetworkId == null) { + logger.warn("Skipping egress firewall rule {} as guestNetworkId is null", rule.getUuid()); + continue; + } final NetworkVO network = _networkDao.findById(guestNetworkId); final NetworkOfferingVO offering = _networkOfferingDao.findById(network.getNetworkOfferingId()); defaultEgressPolicy = offering.isEgressDefaultPolicy(); @@ -583,11 +602,14 @@ public void createFirewallRulesCommands(final List rules } } - final SetFirewallRulesCommand cmd = new SetFirewallRulesCommand(rulesTO); + final SetFirewallRulesCommand cmd = new SetFirewallRulesCommand(rulesTO, vpcId); cmd.setAccessDetail(NetworkElementCommand.ROUTER_IP, _routerControlHelper.getRouterControlIp(router.getId())); if (guestNetworkId != null) { cmd.setAccessDetail(NetworkElementCommand.ROUTER_GUEST_IP, _routerControlHelper.getRouterIpInNetwork(guestNetworkId, router.getId())); } + if (vpcId != null) { + cmd.setAccessDetail(NetworkElementCommand.VPC_ID, String.valueOf(vpcId)); + } cmd.setAccessDetail(NetworkElementCommand.ROUTER_NAME, router.getInstanceName()); final DataCenterVO dcVo = _dcDao.findById(router.getDataCenterId()); cmd.setAccessDetail(NetworkElementCommand.ZONE_NETWORK_TYPE, dcVo.getNetworkType().toString()); @@ -600,6 +622,10 @@ public void createFirewallRulesCommands(final List rules cmds.addCommand(cmd); } + public void createFirewallRulesCommands(final List rules, final VirtualRouter router, final Commands cmds, final Long guestNetworkId) { + createFirewallRulesCommands(rules, router, cmds, guestNetworkId, router.getVpcId()); + } + public void createIpv6FirewallRulesCommands(final List rules, final VirtualRouter router, final Commands cmds, final long guestNetworkId) { final List rulesTO = new ArrayList<>(); String systemRule = null; diff --git a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index 90a26bdb80eb..a166e894be1b 100644 --- a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -2499,7 +2499,7 @@ protected void finalizeNetworkRulesForNetwork(final Commands cmds, final DomainR // Re-apply firewall Egress rules logger.debug("Found " + firewallRulesEgress.size() + " firewall Egress rule(s) to apply as a part of domR " + router + " start."); if (!firewallRulesEgress.isEmpty()) { - _commandSetupHelper.createFirewallRulesCommands(firewallRulesEgress, router, cmds, guestNetworkId); + _commandSetupHelper.createFirewallRulesCommands(firewallRulesEgress, router, cmds, guestNetworkId, router.getVpcId()); } logger.debug(String.format("Found %d Ipv6 firewall rule(s) to apply as a part of domR %s start.", ipv6firewallRules.size(), router)); @@ -2574,7 +2574,7 @@ protected void finalizeNetworkRulesForNetwork(final Commands cmds, final DomainR // Re-apply firewall Ingress rules logger.debug("Found " + firewallRulesIngress.size() + " firewall Ingress rule(s) to apply as a part of domR " + router + " start."); if (!firewallRulesIngress.isEmpty()) { - _commandSetupHelper.createFirewallRulesCommands(firewallRulesIngress, router, cmds, guestNetworkId); + _commandSetupHelper.createFirewallRulesCommands(firewallRulesIngress, router, cmds, guestNetworkId, router.getVpcId()); } // Re-apply port forwarding rules diff --git a/server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java b/server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java index aa772d01fb79..49c2243d1ecf 100644 --- a/server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java +++ b/server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java @@ -660,7 +660,7 @@ private void reapplyVpcFirewallIngressRules(final Commands cmds, final DomainRou } logger.debug("Found {} VPC firewall ingress rule(s) to apply as a part of domR {} start for VPC {}", firewallRulesIngress.size(), domainRouterVO, vpcId); - _commandSetupHelper.createFirewallRulesCommands(firewallRulesIngress, domainRouterVO, cmds, null); + _commandSetupHelper.createFirewallRulesCommands(firewallRulesIngress, domainRouterVO, cmds, null, vpcId); } protected boolean sendNetworkRulesToRouter(final long routerId, final long networkId, final boolean reprogramNetwork) throws ResourceUnavailableException { diff --git a/server/src/main/java/com/cloud/network/rules/FirewallRules.java b/server/src/main/java/com/cloud/network/rules/FirewallRules.java index e995f143e0e8..11922e676e1e 100644 --- a/server/src/main/java/com/cloud/network/rules/FirewallRules.java +++ b/server/src/main/java/com/cloud/network/rules/FirewallRules.java @@ -36,6 +36,7 @@ import com.cloud.network.router.VirtualRouter; import com.cloud.network.rules.FirewallRule.Purpose; import com.cloud.network.rules.LoadBalancerContainer.Scheme; +import com.cloud.network.vpc.Vpc; import com.cloud.utils.net.Ip; public class FirewallRules extends RuleApplier { @@ -50,6 +51,16 @@ public FirewallRules(final Network network, final List r _rules = rules; } + public FirewallRules(final Network network, final Vpc vpc, final List rules) { + super(network, vpc); + _rules = rules; + } + + public FirewallRules(final Vpc vpc, final List rules) { + super(null, vpc); + _rules = rules; + } + @Override public boolean accept(final NetworkTopologyVisitor visitor, final VirtualRouter router) throws ResourceUnavailableException { _router = router; diff --git a/server/src/main/java/com/cloud/network/rules/RuleApplier.java b/server/src/main/java/com/cloud/network/rules/RuleApplier.java index 73c3855361b0..baf4da324845 100644 --- a/server/src/main/java/com/cloud/network/rules/RuleApplier.java +++ b/server/src/main/java/com/cloud/network/rules/RuleApplier.java @@ -22,6 +22,7 @@ import com.cloud.exception.ResourceUnavailableException; import com.cloud.network.Network; import com.cloud.network.router.VirtualRouter; +import com.cloud.network.vpc.Vpc; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; @@ -30,16 +31,27 @@ public abstract class RuleApplier { protected Logger logger = LogManager.getLogger(getClass()); protected Network _network; + protected Vpc _vpc; protected VirtualRouter _router; public RuleApplier(final Network network) { _network = network; + _vpc = null; + } + + public RuleApplier(final Network network, final Vpc vpc) { + _network = network; + _vpc = vpc; } public Network getNetwork() { return _network; } + public Vpc getVpc() { + return _vpc; + } + public VirtualRouter getRouter() { return _router; } diff --git a/server/src/main/java/org/apache/cloudstack/network/topology/BasicNetworkTopology.java b/server/src/main/java/org/apache/cloudstack/network/topology/BasicNetworkTopology.java index a7000f702ec9..46a6886182b0 100644 --- a/server/src/main/java/org/apache/cloudstack/network/topology/BasicNetworkTopology.java +++ b/server/src/main/java/org/apache/cloudstack/network/topology/BasicNetworkTopology.java @@ -65,6 +65,7 @@ import com.cloud.network.vpc.NetworkACLItem; import com.cloud.network.vpc.PrivateGateway; import com.cloud.network.vpc.StaticRouteProfile; +import com.cloud.network.vpc.Vpc; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.DomainRouterVO; import com.cloud.vm.NicProfile; @@ -228,6 +229,26 @@ public boolean applyFirewallRules(final Network network, final List(firewallRules)); } + @Override + public boolean applyFirewallRulesInVPC(final Vpc vpc, final List rules, final VirtualRouter router) + throws ResourceUnavailableException { + if (rules == null || rules.isEmpty()) { + logger.debug("No firewall rules to be applied for vpc {}", vpc); + return true; + } + + logger.debug("APPLYING FIREWALL RULES"); + + final String typeString = "firewall rules"; + final boolean isPodLevelException = false; + final boolean failWhenDisconnect = false; + final Long podId = null; + + final FirewallRules firewallRules = new FirewallRules(vpc, rules); + + return applyRulesInVPC(vpc, router, typeString, isPodLevelException, podId, failWhenDisconnect, new RuleApplierWrapper(firewallRules)); + } + @Override public boolean applyStaticNats(final Network network, final List rules, final VirtualRouter router) throws ResourceUnavailableException { if (rules == null || rules.isEmpty()) { @@ -444,6 +465,102 @@ public boolean applyRules(final Network network, final VirtualRouter router, fin return result; } + @Override + public boolean applyRulesInVPC(final Vpc vpc, final VirtualRouter router, final String typeString, final boolean isPodLevelException, final Long podId, + final boolean failWhenDisconnect, final RuleApplierWrapper ruleApplierWrapper) throws ResourceUnavailableException { + + if (vpc == null) { + throw new CloudRuntimeException("Unable to apply " + typeString + " because VPC is null"); + } + + if (router == null) { + logger.warn("Unable to apply {}, virtual router doesn't exist in vpc {}", typeString, vpc); + final Long dcId = vpc.getZoneId(); + throw new ResourceUnavailableException("Unable to apply " + typeString, DataCenter.class, dcId); + } + + final RuleApplier ruleApplier = ruleApplierWrapper.getRuleType(); + + final Long dcId = vpc.getZoneId(); + final DataCenter dc = _dcDao.findById(dcId); + final boolean isZoneBasic = dc.getNetworkType() == NetworkType.Basic; + + // isPodLevelException and podId is only used for basic zone + assert !(!isZoneBasic && isPodLevelException || isZoneBasic && isPodLevelException && podId == null); + + final List connectedRouters = new ArrayList(); + final List disconnectedRouters = new ArrayList(); + boolean result = true; + final String msg = "Unable to apply " + typeString + " on disconnected router "; + if (router.getState() == State.Running) { + logger.debug("Applying {} in vpc {}", typeString, vpc); + + if (router.isStopPending()) { + if (_hostDao.findById(router.getHostId()).getState() == Status.Up) { + throw new ResourceUnavailableException("Unable to process due to the stop pending router " + router.getInstanceName() + + " haven't been stopped after it's host coming back!", DataCenter.class, router.getDataCenterId()); + } + logger.debug("Router {} is stop pending, so not sending apply {} commands to the backend", router, typeString); + return false; + } + + try { + result = ruleApplier.accept(getVisitor(), router); + connectedRouters.add(router); + } catch (final AgentUnavailableException e) { + logger.warn("{}{}", msg, router, e); + disconnectedRouters.add(router); + } + + // If rules fail to apply on one domR and not due to + // disconnection, no need to proceed with the rest + if (!result) { + if (isZoneBasic && isPodLevelException) { + throw new ResourceUnavailableException("Unable to apply " + typeString + " on router ", Pod.class, podId); + } + throw new ResourceUnavailableException("Unable to apply " + typeString + " on router ", DataCenter.class, router.getDataCenterId()); + } + + } else if (router.getState() == State.Stopped || router.getState() == State.Stopping) { + logger.debug("Router {} is in {}, so not sending apply {} commands to the backend", router, router.getState(), typeString); + } else { + logger.warn("Unable to apply " + typeString + ", virtual router is not in the right state " + router.getState()); + if (isZoneBasic && isPodLevelException) { + throw new ResourceUnavailableException("Unable to apply " + typeString + ", virtual router is not in the right state", Pod.class, podId); + } + throw new ResourceUnavailableException("Unable to apply " + typeString + ", virtual router is not in the right state", DataCenter.class, router.getDataCenterId()); + } + + if (!connectedRouters.isEmpty()) { + // Shouldn't we include this check inside the method? + if (!isZoneBasic && !disconnectedRouters.isEmpty()) { + // These disconnected redundant virtual routers are out of sync + // now, stop them for synchronization + for (final VirtualRouter virtualRouter : disconnectedRouters) { + // If we have at least 1 disconnected redundant router, callhandleSingleWorkingRedundantRouter(). + if (virtualRouter.getIsRedundantRouter()) { + _networkHelper.handleSingleWorkingRedundantRouter(connectedRouters, disconnectedRouters, msg); + break; + } + } + } + } else if (!disconnectedRouters.isEmpty()) { + if (logger.isDebugEnabled()) { + logger.debug("{}{}", msg, router); + } + if (isZoneBasic && isPodLevelException) { + throw new ResourceUnavailableException(msg, Pod.class, podId); + } + throw new ResourceUnavailableException(msg, DataCenter.class, disconnectedRouters.get(0).getDataCenterId()); + } + + result = true; + if (failWhenDisconnect) { + result = !connectedRouters.isEmpty(); + } + return result; + } + @Override public boolean removeDhcpEntry(Network network, NicProfile nic, VirtualMachineProfile profile, VirtualRouter virtualRouter) throws ResourceUnavailableException { logger.debug("REMOVING DHCP ENTRY RULE"); diff --git a/server/src/main/java/org/apache/cloudstack/network/topology/BasicNetworkVisitor.java b/server/src/main/java/org/apache/cloudstack/network/topology/BasicNetworkVisitor.java index 8702a58ad69e..17a6d826bd17 100644 --- a/server/src/main/java/org/apache/cloudstack/network/topology/BasicNetworkVisitor.java +++ b/server/src/main/java/org/apache/cloudstack/network/topology/BasicNetworkVisitor.java @@ -145,7 +145,9 @@ public boolean visit(final FirewallRules firewall) throws ResourceUnavailableExc } else if (purpose == Purpose.Firewall) { - _commandSetupHelper.createApplyFirewallRulesCommands(rules, router, cmds, network.getId()); + final Long guestNetworkId = network != null ? network.getId() : null; + final Long vpcId = network != null ? network.getVpcId() : router.getVpcId(); + _commandSetupHelper.createApplyFirewallRulesCommands(rules, router, cmds, guestNetworkId, vpcId); return _networkGeneralHelper.sendCommandsToRouter(router, cmds); diff --git a/server/src/main/java/org/apache/cloudstack/network/topology/NetworkTopology.java b/server/src/main/java/org/apache/cloudstack/network/topology/NetworkTopology.java index 176584780fed..58b0acd33d24 100644 --- a/server/src/main/java/org/apache/cloudstack/network/topology/NetworkTopology.java +++ b/server/src/main/java/org/apache/cloudstack/network/topology/NetworkTopology.java @@ -35,6 +35,7 @@ import com.cloud.network.vpc.NetworkACLItem; import com.cloud.network.vpc.PrivateGateway; import com.cloud.network.vpc.StaticRouteProfile; +import com.cloud.network.vpc.Vpc; import com.cloud.vm.DomainRouterVO; import com.cloud.vm.NicProfile; import com.cloud.vm.VirtualMachineProfile; @@ -72,6 +73,8 @@ boolean applyUserData(final Network network, final NicProfile nic, final Virtual boolean applyFirewallRules(final Network network, final List rules, final VirtualRouter router) throws ResourceUnavailableException; + boolean applyFirewallRulesInVPC(final Vpc vpc, final List rules, final VirtualRouter router) throws ResourceUnavailableException; + boolean applyStaticNats(final Network network, final List rules, final VirtualRouter router) throws ResourceUnavailableException; boolean associatePublicIP(final Network network, final List ipAddress, final VirtualRouter router) throws ResourceUnavailableException; @@ -89,6 +92,9 @@ boolean saveUserDataToRouter(final Network network, final NicProfile nic, final boolean applyRules(final Network network, final VirtualRouter router, final String typeString, final boolean isPodLevelException, final Long podId, final boolean failWhenDisconnect, RuleApplierWrapper ruleApplier) throws ResourceUnavailableException; + boolean applyRulesInVPC(final Vpc vpc, final VirtualRouter router, final String typeString, final boolean isPodLevelException, final Long podId, + final boolean failWhenDisconnect, RuleApplierWrapper ruleApplier) throws ResourceUnavailableException; + boolean removeDhcpEntry(final Network network, final NicProfile nic, final VirtualMachineProfile profile, final VirtualRouter virtualRouter) throws ResourceUnavailableException; boolean applyBgpPeers(final Network network, final List bpgPeers, final VirtualRouter virtualRouter) throws ResourceUnavailableException; diff --git a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java index c7b6e851925c..d12419b66e0a 100644 --- a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java +++ b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java @@ -60,9 +60,10 @@ import com.cloud.network.rules.FirewallRule; import com.cloud.network.rules.StaticNat; import com.cloud.network.rules.StaticNatImpl; +import com.cloud.network.vpc.dao.VpcDao; +import com.cloud.network.vpc.VpcVO; import com.cloud.network.vpc.VpcManager; import com.cloud.dc.dao.VlanDao; -import com.cloud.dc.VlanVO; import com.cloud.offerings.NetworkOfferingVO; import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.user.AccountVO; @@ -115,6 +116,9 @@ public class IpAddressManagerTest { @Mock VpcManager vpcMgr; + @Mock + VpcDao vpcDao; + @Mock VlanDao vlanDao; @@ -543,7 +547,7 @@ public void applyRulesNonVpcRuleStillWorksViaNetworkId() throws ResourceUnavaila assertTrue(result); verify(networkDao).findById(networkId); - verify(applier).applyRules(network, FirewallRule.Purpose.Firewall, Collections.singletonList(rule)); + verify(applier).applyRules(network, null, FirewallRule.Purpose.Firewall, Collections.singletonList(rule)); } /** @@ -553,31 +557,20 @@ public void applyRulesNonVpcRuleStillWorksViaNetworkId() throws ResourceUnavaila @Test public void applyRulesVpcRuleResolvesNetworkViaVpcManager() throws ResourceUnavailableException { long vpcId = 20L; - long resolvedNetworkId = 30L; - NetworkVO resolvedNetwork = mock(NetworkVO.class); - when(resolvedNetwork.getId()).thenReturn(resolvedNetworkId); - when(networkDao.findById(resolvedNetworkId)).thenReturn(resolvedNetwork); - - doReturn(Collections.singletonList(resolvedNetwork)).when(vpcMgr).getVpcNetworks(vpcId); + VpcVO vpc = mock(VpcVO.class); + when(vpcDao.findById(vpcId)).thenReturn(vpc); FirewallRule rule = makeRule(null, vpcId, FirewallRule.Purpose.Firewall, FirewallRule.TrafficType.Ingress); NetworkRuleApplier applier = mock(NetworkRuleApplier.class); - IPAddressVO vpcIp = mock(IPAddressVO.class); - when(vpcIp.getVlanId()).thenReturn(1L); - VlanVO vlan = mock(VlanVO.class); - when(ipAddressDao.listByAssociatedVpc(vpcId, null)).thenReturn(Collections.singletonList(vpcIp)); - when(vlanDao.findById(1L)).thenReturn(vlan); - stubIpAssocHelpers(); boolean result = ipAddressManager.applyRules( Collections.singletonList(rule), FirewallRule.Purpose.Firewall, applier, false); assertTrue(result); - verify(vpcMgr).getVpcNetworks(vpcId); - verify(ipAddressDao).listByAssociatedVpc(vpcId, null); - verify(applier).applyRules(resolvedNetwork, FirewallRule.Purpose.Firewall, Collections.singletonList(rule)); + verify(vpcDao).findById(vpcId); + verify(applier).applyRules(null, vpc, FirewallRule.Purpose.Firewall, Collections.singletonList(rule)); } @@ -587,11 +580,8 @@ public void applyRulesVpcRuleResolvesNetworkViaVpcManager() throws ResourceUnava @Test public void applyRulesVpcEgressFirewallRuleSkipsIpCollection() throws ResourceUnavailableException { long vpcId = 20L; - long resolvedNetworkId = 30L; - NetworkVO resolvedNetwork = mock(NetworkVO.class); - when(resolvedNetwork.getId()).thenReturn(resolvedNetworkId); - when(networkDao.findById(resolvedNetworkId)).thenReturn(resolvedNetwork); - doReturn(Collections.singletonList(resolvedNetwork)).when(vpcMgr).getVpcNetworks(vpcId); + VpcVO vpc = mock(VpcVO.class); + when(vpcDao.findById(vpcId)).thenReturn(vpc); FirewallRule rule = makeRule(null, vpcId, FirewallRule.Purpose.Firewall, FirewallRule.TrafficType.Egress); NetworkRuleApplier applier = mock(NetworkRuleApplier.class); @@ -603,7 +593,7 @@ public void applyRulesVpcEgressFirewallRuleSkipsIpCollection() throws ResourceUn assertTrue(result); verify(ipAddressDao, never()).listByAssociatedVpc(anyLong(), any()); - verify(applier).applyRules(resolvedNetwork, FirewallRule.Purpose.Firewall, Collections.singletonList(rule)); + verify(applier).applyRules(null, vpc, FirewallRule.Purpose.Firewall, Collections.singletonList(rule)); } /** @@ -613,17 +603,8 @@ public void applyRulesVpcEgressFirewallRuleSkipsIpCollection() throws ResourceUn @Test public void applyRulesVpcIngressRuleCollectsIpsFromVpcNotNetwork() throws ResourceUnavailableException { long vpcId = 20L; - long resolvedNetworkId = 30L; - NetworkVO resolvedNetwork = mock(NetworkVO.class); - when(resolvedNetwork.getId()).thenReturn(resolvedNetworkId); - when(networkDao.findById(resolvedNetworkId)).thenReturn(resolvedNetwork); - doReturn(Collections.singletonList(resolvedNetwork)).when(vpcMgr).getVpcNetworks(vpcId); - - IPAddressVO vpcIp = mock(IPAddressVO.class); - when(vpcIp.getVlanId()).thenReturn(1L); - VlanVO vlan = mock(VlanVO.class); - when(ipAddressDao.listByAssociatedVpc(vpcId, null)).thenReturn(Collections.singletonList(vpcIp)); - when(vlanDao.findById(1L)).thenReturn(vlan); + VpcVO vpc = mock(VpcVO.class); + when(vpcDao.findById(vpcId)).thenReturn(vpc); stubIpAssocHelpers(); @@ -632,7 +613,8 @@ public void applyRulesVpcIngressRuleCollectsIpsFromVpcNotNetwork() throws Resour ipAddressManager.applyRules(Collections.singletonList(rule), FirewallRule.Purpose.Firewall, applier, false); - verify(ipAddressDao).listByAssociatedVpc(vpcId, null); + verify(applier).applyRules(null, vpc, FirewallRule.Purpose.Firewall, Collections.singletonList(rule)); + verify(ipAddressDao, never()).listByAssociatedVpc(vpcId, null); verify(ipAddressDao, never()).listByAssociatedNetwork(anyLong(), any()); } @@ -643,22 +625,13 @@ public void applyRulesVpcIngressRuleCollectsIpsFromVpcNotNetwork() throws Resour @Test public void applyRulesVpcRuleErrorHandlingWithContinueOnErrorTrue() throws ResourceUnavailableException { long vpcId = 20L; - long resolvedNetworkId = 30L; - NetworkVO resolvedNetwork = mock(NetworkVO.class); - when(resolvedNetwork.getId()).thenReturn(resolvedNetworkId); - when(networkDao.findById(resolvedNetworkId)).thenReturn(resolvedNetwork); - doReturn(Collections.singletonList(resolvedNetwork)).when(vpcMgr).getVpcNetworks(vpcId); - - IPAddressVO vpcIp = mock(IPAddressVO.class); - when(vpcIp.getVlanId()).thenReturn(1L); - VlanVO vlan = mock(VlanVO.class); - when(ipAddressDao.listByAssociatedVpc(vpcId, null)).thenReturn(Collections.singletonList(vpcIp)); - when(vlanDao.findById(1L)).thenReturn(vlan); + VpcVO vpc = mock(VpcVO.class); + when(vpcDao.findById(vpcId)).thenReturn(vpc); stubIpAssocHelpers(); NetworkRuleApplier applier = mock(NetworkRuleApplier.class); - when(applier.applyRules(any(), any(), any())).thenThrow(new ResourceUnavailableException("test", Network.class, 0L)); + when(applier.applyRules(any(), any(), any(), any())).thenThrow(new ResourceUnavailableException("test", Network.class, 0L)); FirewallRule rule = makeRule(null, vpcId, FirewallRule.Purpose.Firewall, FirewallRule.TrafficType.Ingress); diff --git a/server/src/test/java/com/cloud/network/element/VpcVirtualRouterElementTest.java b/server/src/test/java/com/cloud/network/element/VpcVirtualRouterElementTest.java index ede1feb74288..a9928c487870 100644 --- a/server/src/test/java/com/cloud/network/element/VpcVirtualRouterElementTest.java +++ b/server/src/test/java/com/cloud/network/element/VpcVirtualRouterElementTest.java @@ -205,12 +205,13 @@ public void testCanHandleFirewallUsesVpcCapability() { final Network network = Mockito.mock(Network.class); when(_networkMdl.getPhysicalNetworkId(network)).thenReturn(1L); + when(network.getId()).thenReturn(200L); when(network.getVpcId()).thenReturn(100L); when(_networkMdl.isProviderEnabledInPhysicalNetwork(1L, Network.Provider.VPCVirtualRouter.getName())).thenReturn(true); - when(_vpcMgr.isProviderSupportServiceInVpc(100L, Network.Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(true); + when(_networkMdl.isProviderSupportServiceInNetwork(200L, Network.Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(true); assertTrue(vpcVirtualRouterElement.canHandle(network, Network.Service.Firewall)); - verify(_vpcMgr).isProviderSupportServiceInVpc(100L, Network.Service.Firewall, Network.Provider.VPCVirtualRouter); - verify(_networkMdl, never()).isProviderSupportServiceInNetwork(network.getId(), Network.Service.Firewall, Network.Provider.VPCVirtualRouter); + verify(_networkMdl).isProviderSupportServiceInNetwork(200L, Network.Service.Firewall, Network.Provider.VPCVirtualRouter); + verify(_vpcMgr, never()).isProviderSupportServiceInVpc(100L, Network.Service.Firewall, Network.Provider.VPCVirtualRouter); } } 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 e3e949ac1d02..e35fb479ba69 100644 --- a/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java +++ b/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java @@ -661,6 +661,10 @@ public void testCreateFirewallRuleRoutesToNonVpcWhenVpcIdNull() throws NetworkRu @Test public void testApplyRulesForVpcNetworkUsesVpcProviderCheck() throws ResourceUnavailableException { + FirewallManagerImpl firewallMgr = new FirewallManagerImpl(); + firewallMgr._networkModel = _networkModel; + firewallMgr._vpcMgr = _vpcMgr; + Network network = Mockito.mock(Network.class); FirewallServiceProvider fwProvider = Mockito.mock(FirewallServiceProvider.class); List rules = new ArrayList<>(); @@ -672,9 +676,9 @@ public void testApplyRulesForVpcNetworkUsesVpcProviderCheck() throws ResourceUna when(fwProvider.getProvider()).thenReturn(Network.Provider.VPCVirtualRouter); when(_vpcMgr.isProviderSupportServiceInVpc(10L, Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(true); when(fwProvider.applyFWRules(Mockito.eq(network), Mockito.anyList())).thenReturn(true); - _firewallMgr._firewallElements = List.of(fwProvider); + firewallMgr._firewallElements = List.of(fwProvider); - boolean result = _firewallMgr.applyRules(network, Purpose.Firewall, rules); + boolean result = firewallMgr.applyRules(network, Purpose.Firewall, rules); Assert.assertTrue(result); verify(_vpcMgr, times(1)).isProviderSupportServiceInVpc(10L, Service.Firewall, Network.Provider.VPCVirtualRouter); @@ -683,6 +687,10 @@ public void testApplyRulesForVpcNetworkUsesVpcProviderCheck() throws ResourceUna @Test public void testApplyRulesForNonVpcNetworkUsesNetworkModelProviderCheck() throws ResourceUnavailableException { + FirewallManagerImpl firewallMgr = new FirewallManagerImpl(); + firewallMgr._networkModel = _networkModel; + firewallMgr._vpcMgr = _vpcMgr; + Network network = Mockito.mock(Network.class); FirewallServiceProvider fwProvider = Mockito.mock(FirewallServiceProvider.class); List rules = new ArrayList<>(); @@ -695,9 +703,9 @@ public void testApplyRulesForNonVpcNetworkUsesNetworkModelProviderCheck() throws when(fwProvider.getProvider()).thenReturn(Network.Provider.VirtualRouter); when(_networkModel.isProviderSupportServiceInNetwork(1L, Service.Firewall, Network.Provider.VirtualRouter)).thenReturn(true); when(fwProvider.applyFWRules(Mockito.eq(network), Mockito.anyList())).thenReturn(true); - _firewallMgr._firewallElements = List.of(fwProvider); + firewallMgr._firewallElements = List.of(fwProvider); - boolean result = _firewallMgr.applyRules(network, Purpose.Firewall, rules); + boolean result = firewallMgr.applyRules(network, Purpose.Firewall, rules); Assert.assertTrue(result); verify(_networkModel, times(1)).isProviderSupportServiceInNetwork(1L, Service.Firewall, Network.Provider.VirtualRouter); From 19d49ab9c61c4c0bd017e74cece6cd3b6636658b Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Fri, 10 Jul 2026 12:57:54 +0530 Subject: [PATCH 31/32] Fix firewall option in network offering creation when vpc is enabled --- ui/src/views/offering/AddNetworkOffering.vue | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/src/views/offering/AddNetworkOffering.vue b/ui/src/views/offering/AddNetworkOffering.vue index 1a89d2db1cb5..c13a3b08bcc6 100644 --- a/ui/src/views/offering/AddNetworkOffering.vue +++ b/ui/src/views/offering/AddNetworkOffering.vue @@ -946,6 +946,9 @@ export default { provider.enabled = self.isVpcCoreProvider(provider.name, svc.name) || !self.isBuiltInNetworkProvider(provider.name) } + if (svc.name === 'Firewall') { + enabledProviders = enabledProviders.filter(name => name !== 'VpcVirtualRouter') + } } else { // *** non-vpc *** provider.enabled = !['InternalLbVm', 'VpcVirtualRouter', 'Nsx', 'Netris'].includes(provider.name) } From 60186e43e5bfcefeafc2cfe4906cf4c676707fe8 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Fri, 10 Jul 2026 13:28:32 +0530 Subject: [PATCH 32/32] Fix UI --- ui/src/views/offering/AddNetworkOffering.vue | 4 ++-- ui/src/views/offering/AddVpcOffering.vue | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/ui/src/views/offering/AddNetworkOffering.vue b/ui/src/views/offering/AddNetworkOffering.vue index c13a3b08bcc6..995b81ce68c6 100644 --- a/ui/src/views/offering/AddNetworkOffering.vue +++ b/ui/src/views/offering/AddNetworkOffering.vue @@ -946,8 +946,8 @@ export default { provider.enabled = self.isVpcCoreProvider(provider.name, svc.name) || !self.isBuiltInNetworkProvider(provider.name) } - if (svc.name === 'Firewall') { - enabledProviders = enabledProviders.filter(name => name !== 'VpcVirtualRouter') + if (svc.name === 'Firewall' && provider.name === 'VpcVirtualRouter') { + provider.enabled = false } } else { // *** non-vpc *** provider.enabled = !['InternalLbVm', 'VpcVirtualRouter', 'Nsx', 'Netris'].includes(provider.name) diff --git a/ui/src/views/offering/AddVpcOffering.vue b/ui/src/views/offering/AddVpcOffering.vue index 1c896d9d38dc..780da6638f8e 100644 --- a/ui/src/views/offering/AddVpcOffering.vue +++ b/ui/src/views/offering/AddVpcOffering.vue @@ -432,6 +432,9 @@ export default { }) }, isVpcCoreProvider (providerName, serviceName) { + if (serviceName === 'Firewall') { + return ['VpcVirtualRouter'].includes(providerName) + } if (['VpcVirtualRouter', 'Netscaler', 'BigSwitchBcf', 'ConfigDrive'].includes(providerName)) { return true }