From 249c8e7df37bbe043aac89048608cfc74084d3b0 Mon Sep 17 00:00:00 2001 From: Marian Fischer Date: Wed, 11 Nov 2026 08:16:47 +0100 Subject: [PATCH] VPC: associate tier public IPs when a public IP of the tier has firewall rules Since #12706 the Firewall service for public IPs of a VPC is provided by the VPC offering; the network offerings of the tiers do not list it. applyIpAssociations passes the services of the public IPs of a tier to VpcVirtualRouterElement.applyIps, which associates the IPs only when it can handle every one of these services on the tier. Once any public IP of the tier has a firewall rule, Firewall is in that set, canHandle(tier, Firewall) is false, and applyIps skips the association without an error: - port forwarding and load balancing rules on a new IP are applied, but the IP never reaches the router, also not on a network restart; - an IP whose last rule is removed stays configured on the router. Check the Firewall service on the VPC in that case, as applyFWRulesInVPC does. Other services and canHandle(network, service) are unchanged. Log when the IP association is skipped. --- .../element/VpcVirtualRouterElement.java | 19 ++++- .../element/VpcVirtualRouterElementTest.java | 83 +++++++++++++++++++ 2 files changed, 101 insertions(+), 1 deletion(-) 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 99f8d8f697b2..0cf147879b0d 100644 --- a/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java +++ b/server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java @@ -539,7 +539,8 @@ public boolean deletePrivateGateway(final PrivateGateway gateway) throws Concurr public boolean applyIps(final Network network, final List ipAddress, final Set services) throws ResourceUnavailableException { boolean canHandle = true; for (final Service service : services) { - if (!canHandle(network, service)) { + if (!canHandleIpService(network, service)) { + logger.debug("{} element can't handle service {} in the network {}; skipping IP association", getName(), service.getName(), network); canHandle = false; break; } @@ -563,6 +564,22 @@ public boolean applyIps(final Network network, final List rules) throws ResourceUnavailableException { boolean result = true; 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 a9928c487870..c9fcf3d21d9c 100644 --- a/server/src/test/java/com/cloud/network/element/VpcVirtualRouterElementTest.java +++ b/server/src/test/java/com/cloud/network/element/VpcVirtualRouterElementTest.java @@ -21,17 +21,21 @@ import com.cloud.exception.ResourceUnavailableException; import com.cloud.network.Network; import com.cloud.network.NetworkModel; +import com.cloud.network.PublicIpAddress; import com.cloud.network.RemoteAccessVpn; import com.cloud.network.VpnUser; +import com.cloud.network.router.VirtualRouter; import com.cloud.network.router.VpcVirtualNetworkApplianceManagerImpl; import com.cloud.network.vpc.Vpc; import com.cloud.network.vpc.VpcManager; +import com.cloud.network.vpc.VpcVO; import com.cloud.network.vpc.dao.VpcDao; import com.cloud.utils.db.EntityManager; import com.cloud.vm.DomainRouterVO; import com.cloud.vm.dao.DomainRouterDao; import org.apache.cloudstack.network.topology.AdvancedNetworkTopology; import org.apache.cloudstack.network.topology.BasicNetworkTopology; +import org.apache.cloudstack.network.topology.NetworkTopology; import org.apache.cloudstack.network.topology.NetworkTopologyContext; import org.junit.Test; import org.junit.runner.RunWith; @@ -41,9 +45,12 @@ import org.mockito.junit.MockitoJUnitRunner; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -214,4 +221,80 @@ public void testCanHandleFirewallUsesVpcCapability() { verify(_networkMdl).isProviderSupportServiceInNetwork(200L, Network.Service.Firewall, Network.Provider.VPCVirtualRouter); verify(_vpcMgr, never()).isProviderSupportServiceInVpc(100L, Network.Service.Firewall, Network.Provider.VPCVirtualRouter); } + + private Network mockVpcTier() { + 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); + return network; + } + + private VpcVO mockVpc(final boolean vpcProvidesFirewall) { + final VpcVO vpc = Mockito.mock(VpcVO.class); + when(vpc.getId()).thenReturn(100L); + when(vpc.getZoneId()).thenReturn(1L); + when(_vpcDao.findById(100L)).thenReturn(vpc); + when(_networkMdl.isProviderEnabledInZone(1L, Network.Provider.VPCVirtualRouter.getName())).thenReturn(true); + when(_vpcMgr.isProviderSupportServiceInVpc(100L, Network.Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(vpcProvidesFirewall); + return vpc; + } + + private Set lbAndFirewall() { + final Set services = new LinkedHashSet<>(); + services.add(Network.Service.Lb); + services.add(Network.Service.Firewall); + return services; + } + + @Test + public void testApplyIpsAssociatesTierIpsWhenTheVpcProvidesFirewall() throws ResourceUnavailableException { + final Network network = mockVpcTier(); + when(network.getDataCenterId()).thenReturn(1L); + when(_networkMdl.isProviderSupportServiceInNetwork(200L, Network.Service.Lb, Network.Provider.VPCVirtualRouter)).thenReturn(true); + when(_networkMdl.isProviderSupportServiceInNetwork(200L, Network.Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(false); + mockVpc(true); + + final DomainRouterVO router = Mockito.mock(DomainRouterVO.class); + when(_routerDao.listByNetworkAndRole(200L, VirtualRouter.Role.VIRTUAL_ROUTER)).thenReturn(List.of(router)); + final DataCenterVO dc = Mockito.mock(DataCenterVO.class); + when(_dcDao.findById(1L)).thenReturn(dc); + final NetworkTopology topology = Mockito.mock(NetworkTopology.class); + when(networkTopologyContext.retrieveNetworkTopology(dc)).thenReturn(topology); + final List ips = List.of(Mockito.mock(PublicIpAddress.class)); + when(topology.associatePublicIP(network, ips, router)).thenReturn(true); + + assertTrue(vpcVirtualRouterElement.applyIps(network, ips, lbAndFirewall())); + verify(topology).associatePublicIP(network, ips, router); + } + + @Test + public void testApplyIpsSkipsAssociationWhenNeitherTierNorVpcProvidesFirewall() throws ResourceUnavailableException { + final Network network = mockVpcTier(); + when(_networkMdl.isProviderSupportServiceInNetwork(200L, Network.Service.Lb, Network.Provider.VPCVirtualRouter)).thenReturn(true); + when(_networkMdl.isProviderSupportServiceInNetwork(200L, Network.Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(false); + mockVpc(false); + + assertTrue(vpcVirtualRouterElement.applyIps(network, List.of(Mockito.mock(PublicIpAddress.class)), lbAndFirewall())); + verify(_routerDao, never()).listByNetworkAndRole(Mockito.anyLong(), Mockito.any()); + } + + @Test + public void testCanHandleIpServiceUsesTheVpcOnlyForFirewall() { + final Network network = mockVpcTier(); + when(_networkMdl.isProviderSupportServiceInNetwork(200L, Network.Service.PortForwarding, Network.Provider.VPCVirtualRouter)).thenReturn(false); + + assertFalse(vpcVirtualRouterElement.canHandleIpService(network, Network.Service.PortForwarding)); + verify(_vpcDao, never()).findById(Mockito.anyLong()); + } + + @Test + public void testCanHandleIpServiceKeepsTierFirewall() { + final Network network = mockVpcTier(); + when(_networkMdl.isProviderSupportServiceInNetwork(200L, Network.Service.Firewall, Network.Provider.VPCVirtualRouter)).thenReturn(true); + + assertTrue(vpcVirtualRouterElement.canHandleIpService(network, Network.Service.Firewall)); + verify(_vpcDao, never()).findById(Mockito.anyLong()); + } }