Skip to content

Commit 59cf032

Browse files
weizhouapacheweizhouapache
committed
NE: check vpc CustomAction provider instead of first tier and cleanup UI
1 parent c8fff1d commit 59cf032

6 files changed

Lines changed: 90 additions & 123 deletions

File tree

‎api/src/main/java/org/apache/cloudstack/extension/ExtensionHelper.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,7 @@ public interface ExtensionHelper {
4343
*/
4444
String NETWORK_SERVICE_CAPABILITIES_DETAIL_KEY = "network.service.capabilities";
4545

46-
Long getExtensionIdForPhysicalNetwork(long physicalNetworkId);
47-
Extension getExtensionForPhysicalNetwork(long physicalNetworkId);
4846
String getExtensionScriptPath(Extension extension);
49-
Map<String, String> getExtensionDetails(long extensionId);
5047

5148
/**
5249
* Finds the extension registered with the given physical network whose name

‎framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java‎

Lines changed: 9 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -461,27 +461,20 @@ protected Extension getExtensionFromResource(ExtensionCustomAction.ResourceType
461461
// Use provider-based lookup: match the network's service-map providers
462462
// against extension names registered on the physical network.
463463
// This correctly handles multiple different extensions on the same physical network.
464-
List<String> providers = networkServiceMapDao.getDistinctProviders(network.getId());
465-
if (CollectionUtils.isNotEmpty(providers)) {
466-
for (String providerName : providers) {
467-
Extension ext = getExtensionForPhysicalNetworkAndProvider(physicalNetworkId, providerName);
468-
if (ext != null) {
469-
return ext;
470-
}
464+
String providerName = networkServiceMapDao.getProviderForServiceInNetwork(network.getId(), Service.CustomAction);
465+
if (providerName != null) {
466+
Extension ext = getExtensionForPhysicalNetworkAndProvider(physicalNetworkId, providerName);
467+
if (ext != null) {
468+
return ext;
471469
}
472470
}
473471
return null;
474472
} else if (resourceType == ExtensionCustomAction.ResourceType.Vpc) {
475473
Vpc vpc = (Vpc) object;
476-
// Find extension via the VPC's tier networks
477-
List<NetworkVO> tierNetworks = networkDao.listByVpc(vpc.getId());
478-
if (CollectionUtils.isNotEmpty(tierNetworks)) {
479-
for (NetworkVO tierNetwork : tierNetworks) {
480-
Extension ext = getExtensionFromResource(ExtensionCustomAction.ResourceType.Network, tierNetwork.getUuid());
481-
if (ext != null) {
482-
return ext;
483-
}
484-
}
474+
// Find extension via the VPC's CustomAction service provider
475+
String providerName = vpcServiceMapDao.getProviderForServiceInVpc(vpc.getId(), Service.CustomAction);
476+
if (providerName != null) {
477+
return extensionDao.findByName(providerName);
485478
}
486479
return null;
487480
}
@@ -2386,26 +2379,6 @@ public List<String> getExtensionReservedResourceDetails(long extensionId) {
23862379
return reservedDetails;
23872380
}
23882381

2389-
@Override
2390-
public Long getExtensionIdForPhysicalNetwork(long physicalNetworkId) {
2391-
// Returns the first (primary) extension for backward compatibility
2392-
List<ExtensionResourceMapVO> maps = extensionResourceMapDao.listByResourceIdAndType(physicalNetworkId,
2393-
ExtensionResourceMap.ResourceType.PhysicalNetwork);
2394-
if (maps == null || maps.isEmpty()) {
2395-
return null;
2396-
}
2397-
return maps.get(0).getExtensionId();
2398-
}
2399-
2400-
@Override
2401-
public Extension getExtensionForPhysicalNetwork(long physicalNetworkId) {
2402-
Long extensionId = getExtensionIdForPhysicalNetwork(physicalNetworkId);
2403-
if (extensionId == null) {
2404-
return null;
2405-
}
2406-
return extensionDao.findById(extensionId);
2407-
}
2408-
24092382
@Override
24102383
public boolean start() {
24112384
long pathStateCheckInterval = PathStateCheckInterval.value();
@@ -2504,11 +2477,6 @@ public String getExtensionScriptPath(Extension extension) {
25042477
return externalProvisioner.getExtensionPath(extension.getRelativePath());
25052478
}
25062479

2507-
@Override
2508-
public Map<String, String> getExtensionDetails(long extensionId) {
2509-
return extensionDetailsDao.listDetailsKeyPairs(extensionId);
2510-
}
2511-
25122480
@Override
25132481
public Extension getExtensionForPhysicalNetworkAndProvider(long physicalNetworkId, String providerName) {
25142482
if (StringUtils.isBlank(providerName)) {

‎framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/network/NetworkExtensionElement.java‎

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@
119119
import org.apache.cloudstack.extension.ExtensionHelper;
120120
import org.apache.cloudstack.extension.NetworkCustomActionProvider;
121121
import org.apache.cloudstack.resourcedetail.dao.VpcDetailsDao;
122+
import org.apache.commons.lang3.StringUtils;
122123

123124
import java.nio.charset.StandardCharsets;
124125
import java.util.Base64;
@@ -380,7 +381,7 @@ protected Extension resolveExtension(Network network) {
380381
}
381382
}
382383
}
383-
return extensionHelper.getExtensionForPhysicalNetwork(physicalNetworkId);
384+
return null;
384385
}
385386

386387
protected boolean canHandle(Network network, Service service) {
@@ -1264,13 +1265,6 @@ protected File resolveScriptFile(Network network, Extension extension) {
12641265
}
12651266

12661267
File extensionDir = new File(extensionPath);
1267-
1268-
// <extensionPath>/<extensionName>.sh (preferred convention)
1269-
File namedScript = new File(extensionDir, extension.getName() + ".sh");
1270-
if (namedScript.exists() && namedScript.canExecute()) {
1271-
return namedScript;
1272-
}
1273-
// <extensionPath> itself is the script file
12741268
if (extensionDir.isFile() && extensionDir.canExecute()) {
12751269
return extensionDir;
12761270
}
@@ -2256,15 +2250,12 @@ protected Pair<Long, Extension> resolveExtensionForVpc(Vpc vpc) {
22562250
if (physNetworks == null || physNetworks.isEmpty()) {
22572251
return null;
22582252
}
2259-
for (PhysicalNetworkVO pn : physNetworks) {
2260-
Extension ext;
2261-
if (providerName != null && !providerName.isBlank()) {
2262-
ext = extensionHelper.getExtensionForPhysicalNetworkAndProvider(pn.getId(), providerName);
2263-
} else {
2264-
ext = extensionHelper.getExtensionForPhysicalNetwork(pn.getId());
2265-
}
2266-
if (ext != null) {
2267-
return new Pair<>(pn.getId(), ext);
2253+
if (StringUtils.isNotBlank(providerName)) {
2254+
for (PhysicalNetworkVO pn : physNetworks) {
2255+
Extension ext = extensionHelper.getExtensionForPhysicalNetworkAndProvider(pn.getId(), providerName);
2256+
if (ext != null) {
2257+
return new Pair<>(pn.getId(), ext);
2258+
}
22682259
}
22692260
}
22702261
return null;
@@ -2294,10 +2285,6 @@ protected File resolveScriptFileForVpc(Long physicalNetworkId, Extension extensi
22942285
throw new CloudRuntimeException("Could not resolve path for extension " + extension.getName());
22952286
}
22962287
File extensionDir = new File(extensionPath);
2297-
File namedScript = new File(extensionDir, extension.getName() + ".sh");
2298-
if (namedScript.exists() && namedScript.canExecute()) {
2299-
return namedScript;
2300-
}
23012288
if (extensionDir.isFile() && extensionDir.canExecute()) {
23022289
return extensionDir;
23032290
}

‎framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java‎

Lines changed: 51 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -2572,37 +2572,9 @@ public void addInbuiltExtensionReservedResourceDetailsAddedDetails() {
25722572
}
25732573

25742574
// -----------------------------------------------------------------------
2575-
// Tests for ExtensionHelper methods (external network device support)
2575+
// Tests for network custom action behavior
25762576
// -----------------------------------------------------------------------
25772577

2578-
@Test
2579-
public void getExtensionForPhysicalNetworkReturnsExtensionWhenRegistered() {
2580-
long physNetId = 10L;
2581-
long extensionId = 5L;
2582-
ExtensionResourceMapVO mapVO = mock(ExtensionResourceMapVO.class);
2583-
when(mapVO.getExtensionId()).thenReturn(extensionId);
2584-
when(extensionResourceMapDao.listByResourceIdAndType(physNetId,
2585-
ExtensionResourceMap.ResourceType.PhysicalNetwork)).thenReturn(List.of(mapVO));
2586-
ExtensionVO ext = mock(ExtensionVO.class);
2587-
when(extensionDao.findById(extensionId)).thenReturn(ext);
2588-
2589-
Extension result = extensionsManager.getExtensionForPhysicalNetwork(physNetId);
2590-
2591-
assertNotNull(result);
2592-
assertEquals(ext, result);
2593-
}
2594-
2595-
@Test
2596-
public void getExtensionForPhysicalNetworkReturnsNullWhenNotRegistered() {
2597-
long physNetId = 10L;
2598-
when(extensionResourceMapDao.listByResourceIdAndType(physNetId,
2599-
ExtensionResourceMap.ResourceType.PhysicalNetwork)).thenReturn(Collections.emptyList());
2600-
2601-
Extension result = extensionsManager.getExtensionForPhysicalNetwork(physNetId);
2602-
2603-
assertNull(result);
2604-
}
2605-
26062578

26072579
// Helper: a mock object that is both a NetworkElement and a NetworkCustomActionProvider
26082580
interface MockNetworkElement extends NetworkElement, NetworkCustomActionProvider {}
@@ -2684,8 +2656,8 @@ public void getExtensionFromResourceReturnsExtensionForNetworkWithProviderMatch(
26842656
when(network.getPhysicalNetworkId()).thenReturn(5L);
26852657
when(entityManager.findByUuid(eq(Network.class), eq("net-uuid"))).thenReturn(network);
26862658

2687-
List<String> providers = List.of("my-ext-provider");
2688-
when(networkServiceMapDao.getDistinctProviders(10L)).thenReturn(providers);
2659+
String providerName ="my-ext-provider";
2660+
when(networkServiceMapDao.getProviderForServiceInNetwork(10L, Network.Service.CustomAction)).thenReturn(providerName);
26892661

26902662
ExtensionVO ext = mock(ExtensionVO.class);
26912663
doReturn(ext).when(extensionsManager).getExtensionForPhysicalNetworkAndProvider(5L, "my-ext-provider");
@@ -2723,6 +2695,54 @@ public void getExtensionFromResourceReturnsNullForNetworkWithNullPhysicalNetwork
27232695
assertNull(result);
27242696
}
27252697

2698+
// -----------------------------------------------------------------------
2699+
// Tests for getExtensionFromResource with Vpc resource type
2700+
// -----------------------------------------------------------------------
2701+
2702+
@Test
2703+
public void getExtensionFromResourceReturnsExtensionForVpcWithProviderMatch() {
2704+
Vpc vpc = mock(Vpc.class);
2705+
when(vpc.getId()).thenReturn(20L);
2706+
when(entityManager.findByUuid(eq(Vpc.class), eq("vpc-uuid"))).thenReturn(vpc);
2707+
2708+
when(vpcServiceMapDao.getProviderForServiceInVpc(20L, Network.Service.CustomAction)).thenReturn("my-vpc-provider");
2709+
2710+
ExtensionVO ext = mock(ExtensionVO.class);
2711+
when(extensionDao.findByName("my-vpc-provider")).thenReturn(ext);
2712+
2713+
Extension result = extensionsManager.getExtensionFromResource(ExtensionCustomAction.ResourceType.Vpc, "vpc-uuid");
2714+
2715+
assertEquals(ext, result);
2716+
}
2717+
2718+
@Test
2719+
public void getExtensionFromResourceReturnsNullForVpcWithoutProvider() {
2720+
Vpc vpc = mock(Vpc.class);
2721+
when(vpc.getId()).thenReturn(20L);
2722+
when(entityManager.findByUuid(eq(Vpc.class), eq("vpc-uuid"))).thenReturn(vpc);
2723+
2724+
when(vpcServiceMapDao.getProviderForServiceInVpc(20L, Network.Service.CustomAction)).thenReturn(null);
2725+
2726+
Extension result = extensionsManager.getExtensionFromResource(ExtensionCustomAction.ResourceType.Vpc, "vpc-uuid");
2727+
2728+
assertNull(result);
2729+
verify(extensionDao, never()).findByName(anyString());
2730+
}
2731+
2732+
@Test
2733+
public void getExtensionFromResourceReturnsNullForVpcWhenProviderExtensionNotFound() {
2734+
Vpc vpc = mock(Vpc.class);
2735+
when(vpc.getId()).thenReturn(20L);
2736+
when(entityManager.findByUuid(eq(Vpc.class), eq("vpc-uuid"))).thenReturn(vpc);
2737+
2738+
when(vpcServiceMapDao.getProviderForServiceInVpc(20L, Network.Service.CustomAction)).thenReturn("missing-provider");
2739+
when(extensionDao.findByName("missing-provider")).thenReturn(null);
2740+
2741+
Extension result = extensionsManager.getExtensionFromResource(ExtensionCustomAction.ResourceType.Vpc, "vpc-uuid");
2742+
2743+
assertNull(result);
2744+
}
2745+
27262746
// -----------------------------------------------------------------------
27272747
// Tests for listExtensions with resourceId + resourceType (PhysicalNetwork)
27282748
// -----------------------------------------------------------------------
@@ -2825,29 +2845,6 @@ public void registerExtensionWithPhysicalNetworkFailsForNonNetworkOrchestratorTy
28252845
extensionsManager.registerExtensionWithResource(cmd);
28262846
}
28272847

2828-
// -----------------------------------------------------------------------
2829-
// Tests for getExtensionIdForPhysicalNetwork
2830-
// -----------------------------------------------------------------------
2831-
2832-
@Test
2833-
public void getExtensionIdForPhysicalNetworkReturnsIdWhenMapped() {
2834-
ExtensionResourceMapVO mapVO = mock(ExtensionResourceMapVO.class);
2835-
when(mapVO.getExtensionId()).thenReturn(55L);
2836-
when(extensionResourceMapDao.listByResourceIdAndType(10L, ExtensionResourceMap.ResourceType.PhysicalNetwork))
2837-
.thenReturn(List.of(mapVO));
2838-
2839-
Long result = extensionsManager.getExtensionIdForPhysicalNetwork(10L);
2840-
assertEquals(Long.valueOf(55L), result);
2841-
}
2842-
2843-
@Test
2844-
public void getExtensionIdForPhysicalNetworkReturnsNullWhenNotMapped() {
2845-
when(extensionResourceMapDao.listByResourceIdAndType(10L, ExtensionResourceMap.ResourceType.PhysicalNetwork))
2846-
.thenReturn(Collections.emptyList());
2847-
2848-
Long result = extensionsManager.getExtensionIdForPhysicalNetwork(10L);
2849-
assertNull(result);
2850-
}
28512848

28522849
// -----------------------------------------------------------------------
28532850
// Tests for getExtensionForPhysicalNetworkAndProvider

‎ui/src/views/offering/AddNetworkOffering.vue‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,13 @@ export default {
739739
isSupportedServiceObject (obj) {
740740
return (obj !== null && obj !== undefined && Object.keys(obj).length > 0 && obj.constructor === Object && 'provider' in obj)
741741
},
742+
isVpcCoreProvider (providerName) {
743+
return ['VpcVirtualRouter', 'Netscaler', 'BigSwitchBcf', 'ConfigDrive'].includes(providerName)
744+
},
745+
isDynamicExtensionProvider (providerName) {
746+
const knownProviders = ['VirtualRouter', 'VpcVirtualRouter', 'InternalLbVm', 'Netscaler', 'BigSwitchBcf', 'ConfigDrive', 'Nsx', 'Netris']
747+
return !knownProviders.includes(providerName)
748+
},
742749
fetchDomainData () {
743750
const params = {}
744751
params.listAll = true
@@ -917,12 +924,12 @@ export default {
917924
var providers = svc.provider
918925
providers.forEach(function (provider, providerIndex) {
919926
if (self.forVpc) { // *** vpc ***
920-
// For VPC offerings, keep router-specific invalid providers disabled,
921-
// but allow extension/external providers to be selected.
927+
// Keep the known VPC-safe providers allowlisted and only additionally
928+
// enable dynamically discovered extension providers.
922929
if (provider.name === 'InternalLbVm') {
923930
provider.enabled = self.lbType === 'internalLb' && svc.name === 'Lb'
924931
} else {
925-
provider.enabled = !['VirtualRouter', 'Nsx', 'Netris'].includes(provider.name)
932+
provider.enabled = self.isVpcCoreProvider(provider.name) || self.isDynamicExtensionProvider(provider.name)
926933
}
927934
} else { // *** non-vpc ***
928935
provider.enabled = !['InternalLbVm', 'VpcVirtualRouter', 'Nsx', 'Netris'].includes(provider.name)

‎ui/src/views/offering/AddVpcOffering.vue‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,13 @@ export default {
431431
this.zoneLoading = false
432432
})
433433
},
434+
isVpcCoreProvider (providerName) {
435+
return ['VpcVirtualRouter', 'Netscaler', 'BigSwitchBcf', 'ConfigDrive'].includes(providerName)
436+
},
437+
isDynamicExtensionProvider (providerName) {
438+
const knownProviders = ['VirtualRouter', 'VpcVirtualRouter', 'InternalLbVm', 'Netscaler', 'BigSwitchBcf', 'ConfigDrive', 'Nsx', 'Netris']
439+
return !knownProviders.includes(providerName)
440+
},
434441
fetchSupportedServiceData () {
435442
var services = []
436443
if (this.provider === 'NSX') {
@@ -520,6 +527,7 @@ export default {
520527
provider: [{ name: 'VpcVirtualRouter' }]
521528
})
522529
} else {
530+
this.supportedServices = []
523531
this.supportedServiceLoading = true
524532
getAPI('listSupportedNetworkServices').then(json => {
525533
const vpcServices = ['Dhcp', 'Dns', 'Lb', 'Gateway', 'StaticNat', 'SourceNat', 'NetworkACL', 'PortForwarding', 'UserData', 'Vpn', 'Connectivity', 'CustomAction']
@@ -532,7 +540,7 @@ export default {
532540
const providerName = provider.name === 'VirtualRouter' ? 'VpcVirtualRouter' : provider.name
533541
const enabled = providerName === 'InternalLbVm'
534542
? service.name === 'Lb'
535-
: !['VirtualRouter', 'Nsx', 'Netris'].includes(providerName)
543+
: this.isVpcCoreProvider(providerName) || this.isDynamicExtensionProvider(providerName)
536544
return {
537545
name: providerName,
538546
description: providerName,
@@ -558,6 +566,9 @@ export default {
558566
services = services.filter(service => !['SourceNat', 'StaticNat', 'Lb', 'PortForwarding', 'Vpn'].includes(service.name))
559567
}
560568
this.supportedServices = services
569+
}).catch(error => {
570+
this.supportedServices = []
571+
this.$notifyError(error)
561572
}).finally(() => {
562573
this.supportedServiceLoading = false
563574
})

0 commit comments

Comments
 (0)