From 545a467d5391d26a467b6efe8e48a6d3a4c9753b Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 8 Oct 2026 18:23:35 +0200 Subject: [PATCH 1/3] extensions: ignore removed offerings when removing a network service --- .../manager/ExtensionsManagerImpl.java | 19 ++++++-- .../manager/ExtensionsManagerImplTest.java | 44 +++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java index a209acf6442e..589c0f84f56e 100644 --- a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java +++ b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java @@ -140,8 +140,10 @@ import com.cloud.network.dao.PhysicalNetworkDao; import com.cloud.network.dao.PhysicalNetworkVO; import com.cloud.network.vpc.Vpc; +import com.cloud.network.vpc.dao.VpcOfferingDao; import com.cloud.network.vpc.dao.VpcOfferingServiceMapDao; import com.cloud.network.vpc.dao.VpcServiceMapDao; +import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; import com.cloud.org.Cluster; import com.cloud.serializer.GsonHelper; @@ -248,6 +250,12 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana @Inject VpcOfferingServiceMapDao vpcOfferingServiceMapDao; + @Inject + NetworkOfferingDao networkOfferingDao; + + @Inject + VpcOfferingDao vpcOfferingDao; + @Inject NetworkModel networkModel; @@ -1306,10 +1314,13 @@ protected void updateNetworkExtensionServicesOnPhysicalNetworks(ExtensionVO exte return; } for (Service removedService : removedServices) { - boolean usedByNetworkOffering = CollectionUtils.isNotEmpty( - networkOfferingServiceMapDao.listOfferingIdsByServiceAndProvider(removedService, extension.getName())); - boolean usedByVpcOffering = CollectionUtils.isNotEmpty( - vpcOfferingServiceMapDao.listOfferingIdsByServiceAndProvider(removedService, extension.getName())); + // offering service map rows are kept when an offering is deleted, so ignore removed offerings + boolean usedByNetworkOffering = networkOfferingServiceMapDao + .listOfferingIdsByServiceAndProvider(removedService, extension.getName()).stream() + .anyMatch(offeringId -> networkOfferingDao.findById(offeringId) != null); + boolean usedByVpcOffering = vpcOfferingServiceMapDao + .listOfferingIdsByServiceAndProvider(removedService, extension.getName()).stream() + .anyMatch(offeringId -> vpcOfferingDao.findById(offeringId) != null); if (usedByNetworkOffering || usedByVpcOffering) { throw new CloudRuntimeException(String.format( "Cannot remove service %s from extension '%s' as it is used by a network/VPC offering", diff --git a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java index fa8c97742697..9a85840544fa 100644 --- a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java +++ b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java @@ -52,6 +52,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; import org.apache.cloudstack.acl.Role; @@ -139,8 +140,12 @@ import com.cloud.network.dao.PhysicalNetworkVO; import com.cloud.network.element.NetworkElement; import com.cloud.network.vpc.Vpc; +import com.cloud.network.vpc.VpcOfferingVO; +import com.cloud.network.vpc.dao.VpcOfferingDao; import com.cloud.network.vpc.dao.VpcOfferingServiceMapDao; import com.cloud.network.vpc.dao.VpcServiceMapDao; +import com.cloud.offerings.NetworkOfferingVO; +import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; import org.apache.cloudstack.extension.NetworkCustomActionProvider; import com.cloud.org.Cluster; @@ -223,6 +228,12 @@ public class ExtensionsManagerImplTest { @Mock private VpcOfferingServiceMapDao vpcOfferingServiceMapDao; + @Mock + private NetworkOfferingDao networkOfferingDao; + + @Mock + private VpcOfferingDao vpcOfferingDao; + @Before public void setUp() { MockitoAnnotations.openMocks(this); @@ -961,6 +972,7 @@ public void testUpdateExtension_RemovingUsedNetworkServiceThrows() { when(networkOfferingServiceMapDao.listOfferingIdsByServiceAndProvider(Network.Service.StaticNat, "MyExt")) .thenReturn(Collections.singletonList(1L)); + when(networkOfferingDao.findById(1L)).thenReturn(mock(NetworkOfferingVO.class)); extensionsManager.updateExtension(cmd); } @@ -992,10 +1004,42 @@ public void testUpdateExtension_RemovingServiceUsedByVpcOfferingThrows() { .thenReturn(Collections.emptyList()); when(vpcOfferingServiceMapDao.listOfferingIdsByServiceAndProvider(Network.Service.StaticNat, "MyExt")) .thenReturn(Collections.singletonList(1L)); + when(vpcOfferingDao.findById(1L)).thenReturn(mock(VpcOfferingVO.class)); extensionsManager.updateExtension(cmd); } + @Test + public void updateNetworkExtensionServicesOnPhysicalNetworks_IgnoresRemovedOfferings() { + ExtensionVO ext = mock(ExtensionVO.class); + when(ext.getId()).thenReturn(9L); + when(ext.getName()).thenReturn("MyExt"); + + // service map rows still reference offerings 1 (network) and 2 (VPC), but both offerings are removed + when(networkOfferingServiceMapDao.listOfferingIdsByServiceAndProvider(Network.Service.CustomAction, "MyExt")) + .thenReturn(Collections.singletonList(1L)); + when(vpcOfferingServiceMapDao.listOfferingIdsByServiceAndProvider(Network.Service.CustomAction, "MyExt")) + .thenReturn(Collections.singletonList(2L)); + when(networkOfferingDao.findById(1L)).thenReturn(null); + when(vpcOfferingDao.findById(2L)).thenReturn(null); + + when(extensionResourceMapDao.listResourceIdsByExtensionIdAndType(9L, ExtensionResourceMap.ResourceType.PhysicalNetwork)) + .thenReturn(Collections.singletonList(100L)); + PhysicalNetworkServiceProviderVO nsp = mock(PhysicalNetworkServiceProviderVO.class); + when(nsp.getId()).thenReturn(500L); + when(nsp.getEnabledServices()).thenReturn(new ArrayList<>(Collections.singletonList(Network.Service.CustomAction))); + when(physicalNetworkServiceProviderDao.findByServiceProvider(100L, "MyExt")).thenReturn(nsp); + + extensionsManager.updateNetworkExtensionServicesOnPhysicalNetworks(ext, + Set.of(Network.Service.CustomAction), Set.of(Network.Service.UserData)); + + ArgumentCaptor> captor = ArgumentCaptor.forClass(List.class); + verify(nsp).setEnabledServices(captor.capture()); + assertFalse(captor.getValue().contains(Network.Service.CustomAction)); + assertTrue(captor.getValue().contains(Network.Service.UserData)); + verify(physicalNetworkServiceProviderDao).update(500L, nsp); + } + @Test public void testUpdateExtension_UpdatesPhysicalNetworkServicesWhenNotInUse() { UpdateExtensionCmd cmd = mock(UpdateExtensionCmd.class); From 7259bc45a4889ad10e0978ccc88cd80e31e71660 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 9 Nov 2026 15:02:30 +0100 Subject: [PATCH 2/3] no nead for parameterised list in test --- .../framework/extensions/manager/ExtensionsManagerImplTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java index 9a85840544fa..c6e42c3b76b5 100644 --- a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java +++ b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java @@ -1010,6 +1010,7 @@ public void testUpdateExtension_RemovingServiceUsedByVpcOfferingThrows() { } @Test + @SuppressWarnings("unchecked") public void updateNetworkExtensionServicesOnPhysicalNetworks_IgnoresRemovedOfferings() { ExtensionVO ext = mock(ExtensionVO.class); when(ext.getId()).thenReturn(9L); From 3d8d704973e72e2c8f49970f698933d0f55dc343 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Tue, 10 Nov 2026 09:54:30 +0100 Subject: [PATCH 3/3] extensions: removal tests --- .../smoke/test_network_extension_namespace.py | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/test/integration/smoke/test_network_extension_namespace.py b/test/integration/smoke/test_network_extension_namespace.py index f2484d73d8c1..2a43583d4872 100644 --- a/test/integration/smoke/test_network_extension_namespace.py +++ b/test/integration/smoke/test_network_extension_namespace.py @@ -408,6 +408,8 @@ class TestNetworkExtensionNamespace(cloudstackTestCase): test_09 — VPC source NAT IP update without VPC restart test_10 — custom-action smoke for Policy-Based Routing (PBR) actions on VPC tier network test_11 — IPv6 dualstack isolated network: namespace IPv6 address + radvd + test_12 — service of a deleted network offering can be removed from the extension + test_13 — service of a deleted VPC offering can be removed from the extension """ @staticmethod @@ -1252,6 +1254,152 @@ def test_03_extension_enable_disable_and_delete_restriction(self): self.physical_network = None self.logger.info("test_03 PASSED") + def _register_extension_without_hosts(self, ext_name_prefix): + """Create a NetworkOrchestrator extension, register it to the guest + physical network without any hosts and enable its NSP. + + Returns the extension (= provider) name. + """ + self.physical_network = self._get_physical_network() + ext_name = "%s-%s" % (ext_name_prefix, random_gen()) + self.extension = Extension.create( + self.apiclient, + name=ext_name, + type='NetworkOrchestrator', + details=[ + {"network.services": NETWORK_SERVICES}, + {"network.service.capabilities": NETWORK_SERVICE_CAPABILITIES_JSON}, + ] + ) + self.extension.register(self.apiclient, self.physical_network.id, + 'PhysicalNetwork') + provider = self._find_provider(self.physical_network.id, ext_name) + self.assertIsNotNone(provider, + "NSP '%s' not found after registration" % ext_name) + self.provider_id = provider.id + if provider.state != 'Enabled': + self._update_provider_state(provider.id, 'Enabled') + return ext_name + + def _count_service_map_rows(self, map_table, offering_column, + offering_table, offering_uuid, provider): + """Count CustomAction rows in an offering service map for *provider*.""" + rows = self.dbclient.execute( + "SELECT COUNT(*) FROM %s m JOIN %s o ON o.id = m.%s " + "WHERE o.uuid = '%s' AND m.service = 'CustomAction' " + "AND m.provider = '%s'" + % (map_table, offering_table, offering_column, offering_uuid, + provider)) + return int(rows[0][0]) + + def _remove_custom_action_from_extension(self): + """Drop CustomAction from the extension's network.services detail.""" + reduced_services = ",".join( + s for s in NETWORK_SERVICES.split(",") if s != "CustomAction") + self.extension.update( + self.apiclient, + details=[ + {"network.services": reduced_services}, + {"network.service.capabilities": NETWORK_SERVICE_CAPABILITIES_JSON}, + ] + ) + + def _get_extension_services(self): + ext = Extension.list(self.apiclient, id=self.extension.id)[0] + d = (ext.details.__dict__ + if not isinstance(ext.details, dict) else ext.details) + return d["network.services"].split(",") + + def _get_provider_services(self, ext_name): + return self._find_provider(self.physical_network.id, + ext_name).servicelist + + def _assert_custom_action_removable_after_offering_deleted( + self, ext_name, offering, map_table, offering_column, + offering_table): + """Removing CustomAction from the extension must be refused while + *offering* exists and allowed once it has been deleted, even though + deleting the offering leaves its service map rows behind.""" + # While the offering exists the service is in use: refuse removal and + # leave both the extension detail and the NSP untouched. + with self.assertRaises(Exception) as ctx: + self._remove_custom_action_from_extension() + self.assertIn("Cannot remove service CustomAction", str(ctx.exception)) + self.assertIn("CustomAction", self._get_extension_services()) + self.assertIn("CustomAction", self._get_provider_services(ext_name)) + + offering.delete(self.apiclient) + + # Deleting an offering only marks it removed; the service map rows that + # tie CustomAction to this provider are kept. + self.assertEqual( + 1, + self._count_service_map_rows(map_table, offering_column, + offering_table, offering.id, + ext_name), + "Expected the %s row of the deleted offering to remain" + % map_table) + + # The only offering using CustomAction is gone, so removal must succeed. + self._remove_custom_action_from_extension() + self.assertNotIn("CustomAction", self._get_extension_services()) + self.assertNotIn("CustomAction", self._get_provider_services(ext_name)) + + @attr(tags=["advanced", "smoke"], required_hardware="false") + def test_12_remove_service_used_by_deleted_network_offering(self): + """A service whose only network offering was deleted can be removed + from the extension.""" + self.dbclient = self.testClient.getDbConnection() + ext_name = self._register_extension_without_hosts("extnet-rmsvc-nw") + offering = None + try: + svc = ISOLATED_NETWORK_SERVICES + offering = NetworkOffering.create(self.apiclient, { + "name": "ExtNet-RmSvc-%s" % random_gen(), + "displaytext": "ExtNet remove service offering", + "guestiptype": "Isolated", + "traffictype": "GUEST", + "supportedservices": svc, + "serviceProviderList": {s: ext_name for s in svc.split(',')}, + "serviceCapabilityList": { + "SourceNat": {"SupportedSourceNatTypes": "peraccount"}, + }, + }) + self._assert_custom_action_removable_after_offering_deleted( + ext_name, offering, "ntwk_offering_service_map", + "network_offering_id", "network_offerings") + offering = None + finally: + if offering: + offering.delete(self.apiclient) + self._teardown_extension() + self.logger.info("test_12 PASSED") + + @attr(tags=["advanced", "smoke"], required_hardware="false") + def test_13_remove_service_used_by_deleted_vpc_offering(self): + """A service whose only VPC offering was deleted can be removed from + the extension.""" + self.dbclient = self.testClient.getDbConnection() + ext_name = self._register_extension_without_hosts("extnet-rmsvc-vpc") + offering = None + try: + svc = VPC_NETWORK_SERVICES + offering = VpcOffering.create(self.apiclient, { + "name": "ExtNet-RmSvc-VPC-%s" % random_gen(), + "displaytext": "ExtNet remove service VPC offering", + "supportedservices": svc, + "serviceProviderList": {s: ext_name for s in svc.split(',')}, + }) + self._assert_custom_action_removable_after_offering_deleted( + ext_name, offering, "vpc_offering_service_map", + "vpc_offering_id", "vpc_offerings") + offering = None + finally: + if offering: + offering.delete(self.apiclient) + self._teardown_extension() + self.logger.info("test_13 PASSED") + # ------------------------------------------------------------------ # Tests — hardware required # ------------------------------------------------------------------