diff --git a/api/src/main/java/org/apache/cloudstack/extension/ExtensionHelper.java b/api/src/main/java/org/apache/cloudstack/extension/ExtensionHelper.java index 2c373988fea..84105dfe5ce 100644 --- a/api/src/main/java/org/apache/cloudstack/extension/ExtensionHelper.java +++ b/api/src/main/java/org/apache/cloudstack/extension/ExtensionHelper.java @@ -43,10 +43,7 @@ public interface ExtensionHelper { */ String NETWORK_SERVICE_CAPABILITIES_DETAIL_KEY = "network.service.capabilities"; - Long getExtensionIdForPhysicalNetwork(long physicalNetworkId); - Extension getExtensionForPhysicalNetwork(long physicalNetworkId); String getExtensionScriptPath(Extension extension); - Map getExtensionDetails(long extensionId); /** * Finds the extension registered with the given physical network whose name 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 0c605577219..9e5db488dd9 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 @@ -461,27 +461,20 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana // Use provider-based lookup: match the network's service-map providers // against extension names registered on the physical network. // This correctly handles multiple different extensions on the same physical network. - List providers = networkServiceMapDao.getDistinctProviders(network.getId()); - if (CollectionUtils.isNotEmpty(providers)) { - for (String providerName : providers) { - Extension ext = getExtensionForPhysicalNetworkAndProvider(physicalNetworkId, providerName); - if (ext != null) { - return ext; - } + String providerName = networkServiceMapDao.getProviderForServiceInNetwork(network.getId(), Service.CustomAction); + if (providerName != null) { + Extension ext = getExtensionForPhysicalNetworkAndProvider(physicalNetworkId, providerName); + if (ext != null) { + return ext; } } return null; } else if (resourceType == ExtensionCustomAction.ResourceType.Vpc) { Vpc vpc = (Vpc) object; - // Find extension via the VPC's tier networks - List tierNetworks = networkDao.listByVpc(vpc.getId()); - if (CollectionUtils.isNotEmpty(tierNetworks)) { - for (NetworkVO tierNetwork : tierNetworks) { - Extension ext = getExtensionFromResource(ExtensionCustomAction.ResourceType.Network, tierNetwork.getUuid()); - if (ext != null) { - return ext; - } - } + // Find extension via the VPC's CustomAction service provider + String providerName = vpcServiceMapDao.getProviderForServiceInVpc(vpc.getId(), Service.CustomAction); + if (providerName != null) { + return extensionDao.findByName(providerName); } return null; } @@ -2386,26 +2379,6 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana return reservedDetails; } - @Override - public Long getExtensionIdForPhysicalNetwork(long physicalNetworkId) { - // Returns the first (primary) extension for backward compatibility - List maps = extensionResourceMapDao.listByResourceIdAndType(physicalNetworkId, - ExtensionResourceMap.ResourceType.PhysicalNetwork); - if (maps == null || maps.isEmpty()) { - return null; - } - return maps.get(0).getExtensionId(); - } - - @Override - public Extension getExtensionForPhysicalNetwork(long physicalNetworkId) { - Long extensionId = getExtensionIdForPhysicalNetwork(physicalNetworkId); - if (extensionId == null) { - return null; - } - return extensionDao.findById(extensionId); - } - @Override public boolean start() { long pathStateCheckInterval = PathStateCheckInterval.value(); @@ -2504,11 +2477,6 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana return externalProvisioner.getExtensionPath(extension.getRelativePath()); } - @Override - public Map getExtensionDetails(long extensionId) { - return extensionDetailsDao.listDetailsKeyPairs(extensionId); - } - @Override public Extension getExtensionForPhysicalNetworkAndProvider(long physicalNetworkId, String providerName) { if (StringUtils.isBlank(providerName)) { diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/network/NetworkExtensionElement.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/network/NetworkExtensionElement.java index 3ad4a661c5e..d8076839043 100644 --- a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/network/NetworkExtensionElement.java +++ b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/network/NetworkExtensionElement.java @@ -119,6 +119,7 @@ import org.apache.cloudstack.extension.Extension; import org.apache.cloudstack.extension.ExtensionHelper; import org.apache.cloudstack.extension.NetworkCustomActionProvider; import org.apache.cloudstack.resourcedetail.dao.VpcDetailsDao; +import org.apache.commons.lang3.StringUtils; import java.nio.charset.StandardCharsets; import java.util.Base64; @@ -380,7 +381,7 @@ public class NetworkExtensionElement extends AdapterBase implements } } } - return extensionHelper.getExtensionForPhysicalNetwork(physicalNetworkId); + return null; } protected boolean canHandle(Network network, Service service) { @@ -1264,13 +1265,6 @@ public class NetworkExtensionElement extends AdapterBase implements } File extensionDir = new File(extensionPath); - - // /.sh (preferred convention) - File namedScript = new File(extensionDir, extension.getName() + ".sh"); - if (namedScript.exists() && namedScript.canExecute()) { - return namedScript; - } - // itself is the script file if (extensionDir.isFile() && extensionDir.canExecute()) { return extensionDir; } @@ -2256,15 +2250,12 @@ public class NetworkExtensionElement extends AdapterBase implements if (physNetworks == null || physNetworks.isEmpty()) { return null; } - for (PhysicalNetworkVO pn : physNetworks) { - Extension ext; - if (providerName != null && !providerName.isBlank()) { - ext = extensionHelper.getExtensionForPhysicalNetworkAndProvider(pn.getId(), providerName); - } else { - ext = extensionHelper.getExtensionForPhysicalNetwork(pn.getId()); - } - if (ext != null) { - return new Pair<>(pn.getId(), ext); + if (StringUtils.isNotBlank(providerName)) { + for (PhysicalNetworkVO pn : physNetworks) { + Extension ext = extensionHelper.getExtensionForPhysicalNetworkAndProvider(pn.getId(), providerName); + if (ext != null) { + return new Pair<>(pn.getId(), ext); + } } } return null; @@ -2294,10 +2285,6 @@ public class NetworkExtensionElement extends AdapterBase implements throw new CloudRuntimeException("Could not resolve path for extension " + extension.getName()); } File extensionDir = new File(extensionPath); - File namedScript = new File(extensionDir, extension.getName() + ".sh"); - if (namedScript.exists() && namedScript.canExecute()) { - return namedScript; - } if (extensionDir.isFile() && extensionDir.canExecute()) { return extensionDir; } 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 3d5b88c40a9..95af2317c88 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 @@ -2572,37 +2572,9 @@ public class ExtensionsManagerImplTest { } // ----------------------------------------------------------------------- - // Tests for ExtensionHelper methods (external network device support) + // Tests for network custom action behavior // ----------------------------------------------------------------------- - @Test - public void getExtensionForPhysicalNetworkReturnsExtensionWhenRegistered() { - long physNetId = 10L; - long extensionId = 5L; - ExtensionResourceMapVO mapVO = mock(ExtensionResourceMapVO.class); - when(mapVO.getExtensionId()).thenReturn(extensionId); - when(extensionResourceMapDao.listByResourceIdAndType(physNetId, - ExtensionResourceMap.ResourceType.PhysicalNetwork)).thenReturn(List.of(mapVO)); - ExtensionVO ext = mock(ExtensionVO.class); - when(extensionDao.findById(extensionId)).thenReturn(ext); - - Extension result = extensionsManager.getExtensionForPhysicalNetwork(physNetId); - - assertNotNull(result); - assertEquals(ext, result); - } - - @Test - public void getExtensionForPhysicalNetworkReturnsNullWhenNotRegistered() { - long physNetId = 10L; - when(extensionResourceMapDao.listByResourceIdAndType(physNetId, - ExtensionResourceMap.ResourceType.PhysicalNetwork)).thenReturn(Collections.emptyList()); - - Extension result = extensionsManager.getExtensionForPhysicalNetwork(physNetId); - - assertNull(result); - } - // Helper: a mock object that is both a NetworkElement and a NetworkCustomActionProvider interface MockNetworkElement extends NetworkElement, NetworkCustomActionProvider {} @@ -2684,8 +2656,8 @@ public class ExtensionsManagerImplTest { when(network.getPhysicalNetworkId()).thenReturn(5L); when(entityManager.findByUuid(eq(Network.class), eq("net-uuid"))).thenReturn(network); - List providers = List.of("my-ext-provider"); - when(networkServiceMapDao.getDistinctProviders(10L)).thenReturn(providers); + String providerName ="my-ext-provider"; + when(networkServiceMapDao.getProviderForServiceInNetwork(10L, Network.Service.CustomAction)).thenReturn(providerName); ExtensionVO ext = mock(ExtensionVO.class); doReturn(ext).when(extensionsManager).getExtensionForPhysicalNetworkAndProvider(5L, "my-ext-provider"); @@ -2723,6 +2695,54 @@ public class ExtensionsManagerImplTest { assertNull(result); } + // ----------------------------------------------------------------------- + // Tests for getExtensionFromResource with Vpc resource type + // ----------------------------------------------------------------------- + + @Test + public void getExtensionFromResourceReturnsExtensionForVpcWithProviderMatch() { + Vpc vpc = mock(Vpc.class); + when(vpc.getId()).thenReturn(20L); + when(entityManager.findByUuid(eq(Vpc.class), eq("vpc-uuid"))).thenReturn(vpc); + + when(vpcServiceMapDao.getProviderForServiceInVpc(20L, Network.Service.CustomAction)).thenReturn("my-vpc-provider"); + + ExtensionVO ext = mock(ExtensionVO.class); + when(extensionDao.findByName("my-vpc-provider")).thenReturn(ext); + + Extension result = extensionsManager.getExtensionFromResource(ExtensionCustomAction.ResourceType.Vpc, "vpc-uuid"); + + assertEquals(ext, result); + } + + @Test + public void getExtensionFromResourceReturnsNullForVpcWithoutProvider() { + Vpc vpc = mock(Vpc.class); + when(vpc.getId()).thenReturn(20L); + when(entityManager.findByUuid(eq(Vpc.class), eq("vpc-uuid"))).thenReturn(vpc); + + when(vpcServiceMapDao.getProviderForServiceInVpc(20L, Network.Service.CustomAction)).thenReturn(null); + + Extension result = extensionsManager.getExtensionFromResource(ExtensionCustomAction.ResourceType.Vpc, "vpc-uuid"); + + assertNull(result); + verify(extensionDao, never()).findByName(anyString()); + } + + @Test + public void getExtensionFromResourceReturnsNullForVpcWhenProviderExtensionNotFound() { + Vpc vpc = mock(Vpc.class); + when(vpc.getId()).thenReturn(20L); + when(entityManager.findByUuid(eq(Vpc.class), eq("vpc-uuid"))).thenReturn(vpc); + + when(vpcServiceMapDao.getProviderForServiceInVpc(20L, Network.Service.CustomAction)).thenReturn("missing-provider"); + when(extensionDao.findByName("missing-provider")).thenReturn(null); + + Extension result = extensionsManager.getExtensionFromResource(ExtensionCustomAction.ResourceType.Vpc, "vpc-uuid"); + + assertNull(result); + } + // ----------------------------------------------------------------------- // Tests for listExtensions with resourceId + resourceType (PhysicalNetwork) // ----------------------------------------------------------------------- @@ -2825,29 +2845,6 @@ public class ExtensionsManagerImplTest { extensionsManager.registerExtensionWithResource(cmd); } - // ----------------------------------------------------------------------- - // Tests for getExtensionIdForPhysicalNetwork - // ----------------------------------------------------------------------- - - @Test - public void getExtensionIdForPhysicalNetworkReturnsIdWhenMapped() { - ExtensionResourceMapVO mapVO = mock(ExtensionResourceMapVO.class); - when(mapVO.getExtensionId()).thenReturn(55L); - when(extensionResourceMapDao.listByResourceIdAndType(10L, ExtensionResourceMap.ResourceType.PhysicalNetwork)) - .thenReturn(List.of(mapVO)); - - Long result = extensionsManager.getExtensionIdForPhysicalNetwork(10L); - assertEquals(Long.valueOf(55L), result); - } - - @Test - public void getExtensionIdForPhysicalNetworkReturnsNullWhenNotMapped() { - when(extensionResourceMapDao.listByResourceIdAndType(10L, ExtensionResourceMap.ResourceType.PhysicalNetwork)) - .thenReturn(Collections.emptyList()); - - Long result = extensionsManager.getExtensionIdForPhysicalNetwork(10L); - assertNull(result); - } // ----------------------------------------------------------------------- // Tests for getExtensionForPhysicalNetworkAndProvider diff --git a/ui/src/views/offering/AddNetworkOffering.vue b/ui/src/views/offering/AddNetworkOffering.vue index 29b506c4f07..96e37915113 100644 --- a/ui/src/views/offering/AddNetworkOffering.vue +++ b/ui/src/views/offering/AddNetworkOffering.vue @@ -739,6 +739,13 @@ export default { isSupportedServiceObject (obj) { return (obj !== null && obj !== undefined && Object.keys(obj).length > 0 && obj.constructor === Object && 'provider' in obj) }, + isVpcCoreProvider (providerName) { + return ['VpcVirtualRouter', 'Netscaler', 'BigSwitchBcf', 'ConfigDrive'].includes(providerName) + }, + isDynamicExtensionProvider (providerName) { + const knownProviders = ['VirtualRouter', 'VpcVirtualRouter', 'InternalLbVm', 'Netscaler', 'BigSwitchBcf', 'ConfigDrive', 'Nsx', 'Netris'] + return !knownProviders.includes(providerName) + }, fetchDomainData () { const params = {} params.listAll = true @@ -917,12 +924,12 @@ export default { var providers = svc.provider providers.forEach(function (provider, providerIndex) { if (self.forVpc) { // *** vpc *** - // For VPC offerings, keep router-specific invalid providers disabled, - // but allow extension/external providers to be selected. + // Keep the known VPC-safe providers allowlisted and only additionally + // enable dynamically discovered extension providers. if (provider.name === 'InternalLbVm') { provider.enabled = self.lbType === 'internalLb' && svc.name === 'Lb' } else { - provider.enabled = !['VirtualRouter', 'Nsx', 'Netris'].includes(provider.name) + provider.enabled = self.isVpcCoreProvider(provider.name) || self.isDynamicExtensionProvider(provider.name) } } 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 96594100308..49c5f6401be 100644 --- a/ui/src/views/offering/AddVpcOffering.vue +++ b/ui/src/views/offering/AddVpcOffering.vue @@ -431,6 +431,13 @@ export default { this.zoneLoading = false }) }, + isVpcCoreProvider (providerName) { + return ['VpcVirtualRouter', 'Netscaler', 'BigSwitchBcf', 'ConfigDrive'].includes(providerName) + }, + isDynamicExtensionProvider (providerName) { + const knownProviders = ['VirtualRouter', 'VpcVirtualRouter', 'InternalLbVm', 'Netscaler', 'BigSwitchBcf', 'ConfigDrive', 'Nsx', 'Netris'] + return !knownProviders.includes(providerName) + }, fetchSupportedServiceData () { var services = [] if (this.provider === 'NSX') { @@ -520,6 +527,7 @@ export default { provider: [{ name: 'VpcVirtualRouter' }] }) } else { + this.supportedServices = [] this.supportedServiceLoading = true getAPI('listSupportedNetworkServices').then(json => { const vpcServices = ['Dhcp', 'Dns', 'Lb', 'Gateway', 'StaticNat', 'SourceNat', 'NetworkACL', 'PortForwarding', 'UserData', 'Vpn', 'Connectivity', 'CustomAction'] @@ -532,7 +540,7 @@ export default { const providerName = provider.name === 'VirtualRouter' ? 'VpcVirtualRouter' : provider.name const enabled = providerName === 'InternalLbVm' ? service.name === 'Lb' - : !['VirtualRouter', 'Nsx', 'Netris'].includes(providerName) + : this.isVpcCoreProvider(providerName) || this.isDynamicExtensionProvider(providerName) return { name: providerName, description: providerName, @@ -558,6 +566,9 @@ export default { services = services.filter(service => !['SourceNat', 'StaticNat', 'Lb', 'PortForwarding', 'Vpn'].includes(service.name)) } this.supportedServices = services + }).catch(error => { + this.supportedServices = [] + this.$notifyError(error) }).finally(() => { this.supportedServiceLoading = false })