Skip to content

Commit 089f743

Browse files
sureshanapartisureshanaparti
committed
Config values check & code improvements
1 parent 7519006 commit 089f743

2 files changed

Lines changed: 93 additions & 27 deletions

File tree

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

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -467,9 +467,9 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
467467

468468
private long _defaultPageSize = Long.parseLong(Config.DefaultPageSize.getDefaultValue());
469469
private static final String DOMAIN_NAME_PATTERN = "^((?!-)[A-Za-z0-9-]{1,63}(?<!-)\\.)+[A-Za-z]{1,63}$";
470-
protected Set<String> configValuesForValidation;
471-
private Set<String> weightBasedParametersForValidation;
472-
private Set<String> overprovisioningFactorsForValidation;
470+
private Set<String> configValuesForValidation = new HashSet<String>();
471+
private Set<String> weightBasedParametersForValidation = new HashSet<String>();
472+
private Set<String> overprovisioningFactorsForValidation = new HashSet<String>();
473473

474474
public static final ConfigKey<Boolean> SystemVMUseLocalStorage = new ConfigKey<Boolean>(Boolean.class, "system.vm.use.local.storage", "Advanced", "false",
475475
"Indicates whether to use local storage pools or shared storage pools for system VMs.", false, ConfigKey.Scope.Zone, null);
@@ -530,8 +530,7 @@ public boolean configure(final String name, final Map<String, Object> params) th
530530
return true;
531531
}
532532

533-
private void populateConfigValuesForValidationSet() {
534-
configValuesForValidation = new HashSet<String>();
533+
protected void populateConfigValuesForValidationSet() {
535534
configValuesForValidation.add("event.purge.interval");
536535
configValuesForValidation.add("account.cleanup.interval");
537536
configValuesForValidation.add("alert.wait");
@@ -559,11 +558,10 @@ private void populateConfigValuesForValidationSet() {
559558
configValuesForValidation.add(StorageManager.STORAGE_POOL_CLIENT_MAX_CONNECTIONS.key());
560559
configValuesForValidation.add(UserDataManager.VM_USERDATA_MAX_LENGTH_STRING);
561560
configValuesForValidation.add(UnmanagedVMsManager.RemoteKvmInstanceDisksCopyTimeout.key());
562-
configValuesForValidation.add(UnmanagedVMsManager.ThreadsOnKVMHostToTransferVMwareVMFiles.key());
561+
configValuesForValidation.add(UnmanagedVMsManager.ConvertVmwareInstanceToKvmTimeout.key());
563562
}
564563

565564
private void weightBasedParametersForValidation() {
566-
weightBasedParametersForValidation = new HashSet<String>();
567565
weightBasedParametersForValidation.add(AlertManager.CPUCapacityThreshold.key());
568566
weightBasedParametersForValidation.add(AlertManager.StorageAllocatedCapacityThreshold.key());
569567
weightBasedParametersForValidation.add(AlertManager.StorageCapacityThreshold.key());
@@ -583,11 +581,9 @@ private void weightBasedParametersForValidation() {
583581
weightBasedParametersForValidation.add(CapacityManager.SecondaryStorageCapacityThreshold.key());
584582
weightBasedParametersForValidation.add(ClusterDrsService.ClusterDrsImbalanceThreshold.key());
585583
weightBasedParametersForValidation.add(ClusterDrsService.ClusterDrsImbalanceSkipThreshold.key());
586-
587584
}
588585

589586
private void overProvisioningFactorsForValidation() {
590-
overprovisioningFactorsForValidation = new HashSet<String>();
591587
overprovisioningFactorsForValidation.add(CapacityManager.MemOverprovisioningFactor.key());
592588
overprovisioningFactorsForValidation.add(CapacityManager.CpuOverprovisioningFactor.key());
593589
overprovisioningFactorsForValidation.add(CapacityManager.StorageOverprovisioningFactor.key());
@@ -1174,8 +1170,7 @@ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) thr
11741170
return new Pair<Configuration, String>(_configDao.findByName(name), newValue);
11751171
}
11761172

1177-
private String validateConfigurationValue(final String name, String value, final String scope) {
1178-
1173+
protected String validateConfigurationValue(final String name, String value, final String scope) {
11791174
final ConfigurationVO cfg = _configDao.findByName(name);
11801175
if (cfg == null) {
11811176
s_logger.error("Missing configuration variable " + name + " in configuration table");
@@ -1257,19 +1252,23 @@ private String validateConfigurationValue(final String name, String value, final
12571252
return null;
12581253
}
12591254

1260-
if (type.equals(Integer.class) && NetworkModel.MACIdentifier.key().equalsIgnoreCase(name)) {
1255+
if (type.equals(Integer.class)) {
12611256
try {
12621257
final int val = Integer.parseInt(value);
1263-
//The value need to be between 0 to 255 because the mac generation needs a value of 8 bit
1264-
//0 value is considered as disable.
1265-
if(val < 0 || val > 255){
1266-
throw new InvalidParameterValueException(name+" value should be between 0 and 255. 0 value will disable this feature");
1258+
1259+
if (NetworkModel.MACIdentifier.key().equalsIgnoreCase(name)) {
1260+
//The value need to be between 0 to 255 because the mac generation needs a value of 8 bit
1261+
//0 value is considered as disable.
1262+
if(val < 0 || val > 255){
1263+
throw new InvalidParameterValueException(name + " value should be between 0 and 255. 0 value will disable this feature");
1264+
}
1265+
}
1266+
1267+
if (UnmanagedVMsManager.ThreadsOnKVMHostToTransferVMwareVMFiles.key().equalsIgnoreCase(name)) {
1268+
if (val >= 100) {
1269+
throw new InvalidParameterValueException("Please enter a value less than 100 for the configuration parameter:" + name);
1270+
}
12671271
}
1268-
} catch (final NumberFormatException e) {
1269-
s_logger.error("There was an error trying to parse the integer value for:" + name);
1270-
throw new InvalidParameterValueException("There was an error trying to parse the integer value for:" + name);
1271-
}
1272-
}
12731272

12741273
if (type.equals(Integer.class) && configValuesForValidation.contains(name)) {
12751274
try {
@@ -1288,7 +1287,7 @@ private String validateConfigurationValue(final String name, String value, final
12881287
throw new InvalidParameterValueException("Please enter a value less than 257 for the configuration parameter:" + name);
12891288
}
12901289
}
1291-
if (UserDataManager.VM_USERDATA_MAX_LENGTH_STRING.equalsIgnoreCase(name)) {
1290+
if (VM_USERDATA_MAX_LENGTH_STRING.equalsIgnoreCase(name)) {
12921291
if (val > 1048576) {
12931292
throw new InvalidParameterValueException("Please enter a value less than 1048576 for the configuration parameter:" + name);
12941293
}
@@ -1300,8 +1299,8 @@ private String validateConfigurationValue(final String name, String value, final
13001299
}
13011300
}
13021301
} catch (final NumberFormatException e) {
1303-
s_logger.error("There was an error trying to parse the integer value for:" + name);
1304-
throw new InvalidParameterValueException("There was an error trying to parse the integer value for:" + name);
1302+
s_logger.error("There was an error trying to parse the integer value for configuration parameter: " + name);
1303+
throw new InvalidParameterValueException("There was an error trying to parse the integer value for configuration parameter: " + name);
13051304
}
13061305
}
13071306

@@ -1312,8 +1311,8 @@ private String validateConfigurationValue(final String name, String value, final
13121311
throw new InvalidParameterValueException("Please enter a value between 0 and 1 for the configuration parameter: " + name);
13131312
}
13141313
} catch (final NumberFormatException e) {
1315-
s_logger.error("There was an error trying to parse the float value for:" + name);
1316-
throw new InvalidParameterValueException("There was an error trying to parse the float value for:" + name);
1314+
s_logger.error("There was an error trying to parse the float value for configuration parameter: " + name);
1315+
throw new InvalidParameterValueException("There was an error trying to parse the float value for configuration parameter: " + name);
13171316
}
13181317
}
13191318

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

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
// under the License.
1717
package com.cloud.configuration;
1818

19+
import static org.mockito.Mockito.mock;
20+
import static org.mockito.Mockito.when;
21+
1922
import com.cloud.exception.InvalidParameterValueException;
2023
import com.cloud.storage.StorageManager;
2124
import com.cloud.utils.net.NetUtils;
@@ -31,8 +34,11 @@
3134
import com.cloud.utils.db.EntityManager;
3235
import com.cloud.utils.db.SearchCriteria;
3336
import org.apache.cloudstack.api.command.admin.offering.UpdateDiskOfferingCmd;
37+
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
38+
import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
3439
import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO;
3540
import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao;
41+
import org.apache.cloudstack.vm.UnmanagedVMsManager;
3642
import org.junit.Assert;
3743
import org.junit.Before;
3844
import org.junit.Test;
@@ -47,7 +53,6 @@
4753
import java.util.ArrayList;
4854
import java.util.List;
4955

50-
5156
@RunWith(MockitoJUnitRunner.class)
5257
public class ConfigurationManagerImplTest {
5358
@Mock
@@ -65,6 +70,8 @@ public class ConfigurationManagerImplTest {
6570
@Mock
6671
Domain domainMock;
6772
@Mock
73+
ConfigurationDao configDaoMock;
74+
@Mock
6875
DataCenterDao zoneDaoMock;
6976
@Mock
7077
DomainDao domainDaoMock;
@@ -300,6 +307,66 @@ public void testValidateIpAddressRelatedConfigValuesValidIpRange() {
300307
configurationManagerImplSpy.validateIpAddressRelatedConfigValues("config.iprange", "192.168.1.1-192.168.1.100");
301308
}
302309

310+
@Test
311+
public void testValidateInvalidConfiguration() {
312+
Mockito.doReturn(null).when(configDaoMock).findByName(Mockito.anyString());
313+
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Global.toString());
314+
Assert.assertEquals("Invalid configuration variable.", msg);
315+
}
316+
317+
@Test
318+
public void testValidateInvalidScopeForConfiguration() {
319+
ConfigurationVO cfg = mock(ConfigurationVO.class);
320+
when(cfg.getScope()).thenReturn(ConfigKey.Scope.Account.toString());
321+
Mockito.doReturn(cfg).when(configDaoMock).findByName(Mockito.anyString());
322+
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Domain.toString());
323+
Assert.assertEquals("Invalid scope id provided for the parameter test.config.name", msg);
324+
}
325+
326+
@Test(expected = InvalidParameterValueException.class)
327+
public void testValidateConfig_ThreadsOnKVMHostToTransferVMwareVMFiles_Failure() {
328+
ConfigurationVO cfg = mock(ConfigurationVO.class);
329+
when(cfg.getScope()).thenReturn(ConfigKey.Scope.Global.toString());
330+
ConfigKey<Integer> configKey = UnmanagedVMsManager.ThreadsOnKVMHostToTransferVMwareVMFiles;
331+
Mockito.doReturn(cfg).when(configDaoMock).findByName(Mockito.anyString());
332+
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
333+
configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "100", configKey.scope().toString());
334+
}
335+
336+
@Test
337+
public void testValidateConfig_ThreadsOnKVMHostToTransferVMwareVMFiles_Success() {
338+
ConfigurationVO cfg = mock(ConfigurationVO.class);
339+
when(cfg.getScope()).thenReturn(ConfigKey.Scope.Global.toString());
340+
ConfigKey<Integer> configKey = UnmanagedVMsManager.ThreadsOnKVMHostToTransferVMwareVMFiles;
341+
Mockito.doReturn(cfg).when(configDaoMock).findByName(Mockito.anyString());
342+
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
343+
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "10", configKey.scope().toString());
344+
Assert.assertNull(msg);
345+
}
346+
347+
@Test(expected = InvalidParameterValueException.class)
348+
public void testValidateConfig_ConvertVmwareInstanceToKvmTimeout_Failure() {
349+
ConfigurationVO cfg = mock(ConfigurationVO.class);
350+
when(cfg.getScope()).thenReturn(ConfigKey.Scope.Global.toString());
351+
ConfigKey<Integer> configKey = UnmanagedVMsManager.ConvertVmwareInstanceToKvmTimeout;
352+
Mockito.doReturn(cfg).when(configDaoMock).findByName(Mockito.anyString());
353+
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
354+
configurationManagerImplSpy.populateConfigValuesForValidationSet();
355+
configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "0", configKey.scope().toString());
356+
}
357+
358+
@Test
359+
public void testValidateConfig_ConvertVmwareInstanceToKvmTimeout_Success() {
360+
ConfigurationVO cfg = mock(ConfigurationVO.class);
361+
when(cfg.getScope()).thenReturn(ConfigKey.Scope.Global.toString());
362+
ConfigKey<Integer> configKey = UnmanagedVMsManager.ConvertVmwareInstanceToKvmTimeout;
363+
Mockito.doReturn(cfg).when(configDaoMock).findByName(Mockito.anyString());
364+
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
365+
configurationManagerImplSpy.populateConfigValuesForValidationSet();
366+
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "9", configKey.scope().toString());
367+
Assert.assertNull(msg);
368+
}
369+
303370
@Test
304371
public void validateDomainTestInvalidIdThrowException() {
305372
Mockito.doReturn(null).when(domainDaoMock).findById(invalidId);

0 commit comments

Comments
 (0)