Skip to content

Commit 6ea054c

Browse files
Pearl1594Pearl1594
committed
add some checks to prevent networkmode change when provider is nsx/netris from the source networkmode
1 parent 09cdd31 commit 6ea054c

File tree

3 files changed

+157
-34
lines changed

3 files changed

+157
-34
lines changed

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

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

8301-
String detectedProvider = cmd.getProvider();
8302-
8303-
if (detectedProvider == null || detectedProvider.isEmpty()) {
8304-
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap =
8301+
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap =
83058302
_networkModel.getNetworkOfferingServiceProvidersMap(sourceOfferingId);
83068303

8307-
if (sourceServiceProviderMap.containsKey(Network.Service.NetworkACL)) {
8308-
Set<Network.Provider> networkAclProviders = sourceServiceProviderMap.get(Network.Service.NetworkACL);
8309-
if (networkAclProviders != null && !networkAclProviders.isEmpty()) {
8310-
Network.Provider provider = networkAclProviders.iterator().next();
8311-
if (provider == Network.Provider.Nsx) {
8312-
detectedProvider = "NSX";
8313-
} else if (provider == Network.Provider.Netris) {
8314-
detectedProvider = "Netris";
8315-
}
8316-
}
8317-
}
8318-
}
8304+
validateProvider(sourceOffering, sourceServiceProviderMap, cmd.getProvider(), cmd.getNetworkMode());
8305+
8306+
applySourceOfferingValuesToCloneCmd(cmd, sourceServiceProviderMap, sourceOffering);
83198307

8308+
return createNetworkOffering(cmd);
8309+
}
8310+
8311+
private void validateProvider(NetworkOfferingVO sourceOffering,
8312+
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap,
8313+
String detectedProvider, String networkMode) {
8314+
8315+
detectedProvider = getExternalNetworkProvider(detectedProvider, sourceServiceProviderMap);
83208316
// If this is an NSX/Netris offering, prevent network mode changes
83218317
if (detectedProvider != null && (detectedProvider.equals("NSX") || detectedProvider.equals("Netris"))) {
8322-
String cmdNetworkMode = cmd.getNetworkMode();
8323-
if (cmdNetworkMode != null && sourceOffering.getNetworkMode() != null) {
8324-
if (!cmdNetworkMode.equalsIgnoreCase(sourceOffering.getNetworkMode().toString())) {
8318+
if (networkMode != null && sourceOffering.getNetworkMode() != null) {
8319+
if (!networkMode.equalsIgnoreCase(sourceOffering.getNetworkMode().toString())) {
83258320
throw new InvalidParameterValueException(
8326-
String.format("Cannot change network mode when cloning %s provider network offerings. " +
8327-
"Source offering has network mode '%s', but '%s' was specified. " +
8328-
"The network mode is determined by the provider configuration and cannot be modified.",
8329-
detectedProvider, sourceOffering.getNetworkMode(), cmdNetworkMode));
8321+
String.format("Cannot change network mode when cloning %s provider network offerings. " +
8322+
"Source offering has network mode '%s', but '%s' was specified. ",
8323+
detectedProvider, sourceOffering.getNetworkMode(), networkMode));
83308324
}
83318325
}
83328326
}
8327+
}
83338328

8334-
applySourceOfferingValuesToCloneCmd(cmd, sourceOffering);
8329+
public static String getExternalNetworkProvider(String detectedProvider,
8330+
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap) {
8331+
if (StringUtils.isNotEmpty(detectedProvider)) {
8332+
return detectedProvider;
8333+
}
83358334

8336-
return createNetworkOffering(cmd);
8335+
if (sourceServiceProviderMap == null || sourceServiceProviderMap.isEmpty()) {
8336+
return null;
8337+
}
8338+
8339+
for (Set<Provider> providers : sourceServiceProviderMap.values()) {
8340+
if (CollectionUtils.isEmpty(providers)) {
8341+
continue;
8342+
}
8343+
for (Provider provider : providers) {
8344+
if (provider == Provider.Nsx) {
8345+
return "NSX";
8346+
}
8347+
if (provider == Provider.Netris) {
8348+
return "Netris";
8349+
}
8350+
}
8351+
}
8352+
8353+
return null;
83378354
}
83388355

83398356
/**
@@ -8364,12 +8381,11 @@ private Map<String, Map<String, String>> convertToApiParameterFormat(Map<String,
83648381
return apiFormatMap;
83658382
}
83668383

8367-
private void applySourceOfferingValuesToCloneCmd(CloneNetworkOfferingCmd cmd, NetworkOfferingVO sourceOffering) {
8384+
private void applySourceOfferingValuesToCloneCmd(CloneNetworkOfferingCmd cmd,
8385+
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap,
8386+
NetworkOfferingVO sourceOffering) {
83688387
Long sourceOfferingId = sourceOffering.getId();
83698388

8370-
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap =
8371-
_networkModel.getNetworkOfferingServiceProvidersMap(sourceOfferingId);
8372-
83738389
// Build final services list with add/drop support
83748390
List<String> finalServices = resolveFinalServicesList(cmd, sourceServiceProviderMap);
83758391

‎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)