Skip to content

Commit f02f940

Browse files
Pearl1594Pearl1594
committed
add some checks to prevent networkmode change when provider is nsx/netris from the source networkmode
1 parent ee2e831 commit f02f940

3 files changed

Lines changed: 157 additions & 34 deletions

File tree

‎server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java‎

Lines changed: 45 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8306,42 +8306,59 @@ public NetworkOffering cloneNetworkOffering(final CloneNetworkOfferingCmd cmd) {
83068306
logger.info("Cloning network offering {} (id: {}) to new offering with name: {}",
83078307
sourceOffering.getName(), sourceOfferingId, name);
83088308

8309-
String detectedProvider = cmd.getProvider();
8310-
8311-
if (detectedProvider == null || detectedProvider.isEmpty()) {
8312-
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap =
8309+
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap =
83138310
_networkModel.getNetworkOfferingServiceProvidersMap(sourceOfferingId);
83148311

8315-
if (sourceServiceProviderMap.containsKey(Network.Service.NetworkACL)) {
8316-
Set<Network.Provider> networkAclProviders = sourceServiceProviderMap.get(Network.Service.NetworkACL);
8317-
if (networkAclProviders != null && !networkAclProviders.isEmpty()) {
8318-
Network.Provider provider = networkAclProviders.iterator().next();
8319-
if (provider == Network.Provider.Nsx) {
8320-
detectedProvider = "NSX";
8321-
} else if (provider == Network.Provider.Netris) {
8322-
detectedProvider = "Netris";
8323-
}
8324-
}
8325-
}
8326-
}
8312+
validateProvider(sourceOffering, sourceServiceProviderMap, cmd.getProvider(), cmd.getNetworkMode());
8313+
8314+
applySourceOfferingValuesToCloneCmd(cmd, sourceServiceProviderMap, sourceOffering);
83278315

8316+
return createNetworkOffering(cmd);
8317+
}
8318+
8319+
private void validateProvider(NetworkOfferingVO sourceOffering,
8320+
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap,
8321+
String detectedProvider, String networkMode) {
8322+
8323+
detectedProvider = getExternalNetworkProvider(detectedProvider, sourceServiceProviderMap);
83288324
// If this is an NSX/Netris offering, prevent network mode changes
83298325
if (detectedProvider != null && (detectedProvider.equals("NSX") || detectedProvider.equals("Netris"))) {
8330-
String cmdNetworkMode = cmd.getNetworkMode();
8331-
if (cmdNetworkMode != null && sourceOffering.getNetworkMode() != null) {
8332-
if (!cmdNetworkMode.equalsIgnoreCase(sourceOffering.getNetworkMode().toString())) {
8326+
if (networkMode != null && sourceOffering.getNetworkMode() != null) {
8327+
if (!networkMode.equalsIgnoreCase(sourceOffering.getNetworkMode().toString())) {
83338328
throw new InvalidParameterValueException(
8334-
String.format("Cannot change network mode when cloning %s provider network offerings. " +
8335-
"Source offering has network mode '%s', but '%s' was specified. " +
8336-
"The network mode is determined by the provider configuration and cannot be modified.",
8337-
detectedProvider, sourceOffering.getNetworkMode(), cmdNetworkMode));
8329+
String.format("Cannot change network mode when cloning %s provider network offerings. " +
8330+
"Source offering has network mode '%s', but '%s' was specified. ",
8331+
detectedProvider, sourceOffering.getNetworkMode(), networkMode));
83388332
}
83398333
}
83408334
}
8335+
}
83418336

8342-
applySourceOfferingValuesToCloneCmd(cmd, sourceOffering);
8337+
public static String getExternalNetworkProvider(String detectedProvider,
8338+
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap) {
8339+
if (StringUtils.isNotEmpty(detectedProvider)) {
8340+
return detectedProvider;
8341+
}
83438342

8344-
return createNetworkOffering(cmd);
8343+
if (sourceServiceProviderMap == null || sourceServiceProviderMap.isEmpty()) {
8344+
return null;
8345+
}
8346+
8347+
for (Set<Provider> providers : sourceServiceProviderMap.values()) {
8348+
if (CollectionUtils.isEmpty(providers)) {
8349+
continue;
8350+
}
8351+
for (Provider provider : providers) {
8352+
if (provider == Provider.Nsx) {
8353+
return "NSX";
8354+
}
8355+
if (provider == Provider.Netris) {
8356+
return "Netris";
8357+
}
8358+
}
8359+
}
8360+
8361+
return null;
83458362
}
83468363

83478364
/**
@@ -8372,12 +8389,11 @@ private Map<String, Map<String, String>> convertToApiParameterFormat(Map<String,
83728389
return apiFormatMap;
83738390
}
83748391

8375-
private void applySourceOfferingValuesToCloneCmd(CloneNetworkOfferingCmd cmd, NetworkOfferingVO sourceOffering) {
8392+
private void applySourceOfferingValuesToCloneCmd(CloneNetworkOfferingCmd cmd,
8393+
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap,
8394+
NetworkOfferingVO sourceOffering) {
83768395
Long sourceOfferingId = sourceOffering.getId();
83778396

8378-
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap =
8379-
_networkModel.getNetworkOfferingServiceProvidersMap(sourceOfferingId);
8380-
83818397
// Build final services list with add/drop support
83828398
List<String> finalServices = resolveFinalServicesList(cmd, sourceServiceProviderMap);
83838399

‎server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -835,21 +835,39 @@ public VpcOffering cloneVPCOffering(CloneVPCOfferingCmd cmd) {
835835
VpcOfferingVO vpcOfferingVO = _vpcOffDao.findByUniqueName(name);
836836
if (vpcOfferingVO != null) {
837837
throw new InvalidParameterValueException(String.format("A VPC offering with name %s already exists", name));
838-
839838
}
840839

841840
logger.info("Cloning VPC offering {} (id: {}) to new offering with name: {}",
842841
sourceVpcOffering.getName(), sourceVpcOfferingId, name);
843842

844-
applySourceOfferingValuesToCloneCmd(cmd, sourceVpcOffering);
843+
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap = getVpcOffSvcProvidersMap(sourceVpcOfferingId);
844+
validateProvider(sourceVpcOffering, sourceServiceProviderMap, cmd.getProvider(), cmd.getNetworkMode());
845+
846+
applySourceOfferingValuesToCloneCmd(cmd, sourceServiceProviderMap, sourceVpcOffering);
845847

846848
return createVpcOffering(cmd);
847849
}
848850

849-
private void applySourceOfferingValuesToCloneCmd(CloneVPCOfferingCmd cmd, VpcOffering sourceVpcOffering) {
850-
Long sourceOfferingId = sourceVpcOffering.getId();
851+
private void validateProvider(VpcOffering sourceVpcOffering,
852+
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap,
853+
String provider, String networkMode) {
854+
provider = ConfigurationManagerImpl.getExternalNetworkProvider(provider, sourceServiceProviderMap);
855+
if (provider != null && (provider.equals("NSX") || provider.equals("Netris"))) {
856+
if (networkMode != null && sourceVpcOffering.getNetworkMode() != null) {
857+
if (!networkMode.equalsIgnoreCase(sourceVpcOffering.getNetworkMode().toString())) {
858+
throw new InvalidParameterValueException(
859+
String.format("Cannot change network mode when cloning %s provider VPC offerings. " +
860+
"Source offering has network mode '%s', but '%s' was specified. ",
861+
provider, sourceVpcOffering.getNetworkMode(), networkMode));
862+
}
863+
}
864+
}
865+
}
851866

852-
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap = getVpcOffSvcProvidersMap(sourceOfferingId);
867+
private void applySourceOfferingValuesToCloneCmd(CloneVPCOfferingCmd cmd,
868+
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap,
869+
VpcOffering sourceVpcOffering) {
870+
Long sourceOfferingId = sourceVpcOffering.getId();
853871

854872
List<String> finalServices = resolveFinalServicesList(cmd, sourceServiceProviderMap);
855873

‎server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,15 @@
9090
import org.springframework.test.util.ReflectionTestUtils;
9191

9292
import java.lang.reflect.Field;
93+
import java.lang.reflect.InvocationTargetException;
94+
import java.lang.reflect.Method;
9395
import java.util.ArrayList;
9496
import java.util.Collections;
9597
import java.util.HashMap;
98+
import java.util.HashSet;
9699
import java.util.List;
97100
import java.util.Map;
101+
import java.util.Set;
98102

99103
import static org.mockito.ArgumentMatchers.any;
100104
import static org.mockito.ArgumentMatchers.anyLong;
@@ -1292,4 +1296,89 @@ public void testResolveBooleanParamUsesDefaultWhenRequestParamsIsNull() {
12921296

12931297
Assert.assertFalse(result);
12941298
}
1299+
1300+
@Test
1301+
public void validateProviderDetectsNsxAndPreventsNetworkModeChange() {
1302+
NetworkOfferingVO sourceOffering = mock(NetworkOfferingVO.class);
1303+
when(sourceOffering.getNetworkMode()).thenReturn(NetworkOffering.NetworkMode.NATTED);
1304+
1305+
Map<Network.Service, Set<Network.Provider>> serviceProviderMap = new HashMap<>();
1306+
Set<Network.Provider> providers = new HashSet<>();
1307+
providers.add(Network.Provider.Nsx);
1308+
serviceProviderMap.put(Network.Service.Firewall, providers);
1309+
try {
1310+
Method method = null;
1311+
try {
1312+
method = configurationManagerImplSpy.getClass().getDeclaredMethod("validateProvider", NetworkOfferingVO.class, Map.class, String.class, String.class);
1313+
} catch (NoSuchMethodException nsme) {
1314+
// Method not found; will use ReflectionTestUtils as fallback
1315+
}
1316+
1317+
final String requestedNetworkMode = "routed";
1318+
if (method != null) {
1319+
method.setAccessible(true);
1320+
try {
1321+
method.invoke(configurationManagerImplSpy, sourceOffering, serviceProviderMap, null, requestedNetworkMode);
1322+
Assert.fail("Expected InvalidParameterValueException to be thrown");
1323+
} catch (InvocationTargetException ite) {
1324+
Throwable cause = ite.getCause();
1325+
if (cause instanceof InvalidParameterValueException) {
1326+
return;
1327+
}
1328+
cause.printStackTrace(System.out);
1329+
Assert.fail("Unexpected exception type: " + cause);
1330+
}
1331+
}
1332+
} catch (Exception e) {
1333+
e.printStackTrace(System.out);
1334+
Assert.fail("Test encountered unexpected exception: " + e);
1335+
}
1336+
}
1337+
1338+
@Test
1339+
public void testGetExternalNetworkProviderReturnsDetectedProviderWhenNonEmpty() {
1340+
String detected = "CustomProvider";
1341+
Map<Network.Service, Set<Network.Provider>> serviceProviderMap = new HashMap<>();
1342+
1343+
String result = ConfigurationManagerImpl.getExternalNetworkProvider(detected, serviceProviderMap);
1344+
1345+
Assert.assertEquals(detected, result);
1346+
}
1347+
1348+
@Test
1349+
public void testGetExternalNetworkProviderDetectsNsxFromAnyService() {
1350+
Map<Network.Service, Set<Network.Provider>> serviceProviderMap = new HashMap<>();
1351+
Set<Network.Provider> providers = new HashSet<>();
1352+
providers.add(Network.Provider.Nsx);
1353+
// put NSX under an arbitrary service to ensure method checks all services
1354+
serviceProviderMap.put(Network.Service.Dhcp, providers);
1355+
1356+
String result = ConfigurationManagerImpl.getExternalNetworkProvider(null, serviceProviderMap);
1357+
1358+
Assert.assertEquals("NSX", result);
1359+
}
1360+
1361+
@Test
1362+
public void testGetExternalNetworkProviderDetectsNetrisFromAnyService() {
1363+
Map<Network.Service, Set<Network.Provider>> serviceProviderMap = new HashMap<>();
1364+
Set<Network.Provider> providers = new HashSet<>();
1365+
providers.add(Network.Provider.Netris);
1366+
serviceProviderMap.put(Network.Service.StaticNat, providers);
1367+
1368+
String result = ConfigurationManagerImpl.getExternalNetworkProvider(null, serviceProviderMap);
1369+
1370+
Assert.assertEquals("Netris", result);
1371+
}
1372+
1373+
@Test
1374+
public void testGetExternalNetworkProviderReturnsNullWhenNoExternalProviders() {
1375+
Assert.assertNull(ConfigurationManagerImpl.getExternalNetworkProvider(null, null));
1376+
1377+
Map<Network.Service, Set<Network.Provider>> emptyMap = new HashMap<>();
1378+
Assert.assertNull(ConfigurationManagerImpl.getExternalNetworkProvider(null, emptyMap));
1379+
1380+
Map<Network.Service, Set<Network.Provider>> mapWithEmptySet = new HashMap<>();
1381+
mapWithEmptySet.put(Network.Service.Firewall, Collections.emptySet());
1382+
Assert.assertNull(ConfigurationManagerImpl.getExternalNetworkProvider(null, mapWithEmptySet));
1383+
}
12951384
}

0 commit comments

Comments
 (0)