Skip to content

Commit fa85a75

Browse files
sudo87sudo87
authored
Log previous and new value of configuration when reset/update API is called (#10769)
1 parent 7632814 commit fa85a75

File tree

4 files changed

+61
-36
lines changed

4 files changed

+61
-36
lines changed

‎engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,14 @@ public interface ConfigurationManager {
7777

7878
/**
7979
* Updates a configuration entry with a new value
80-
*
8180
* @param userId
8281
* @param name
82+
* @param category
8383
* @param value
84+
* @param scope
85+
* @param id
8486
*/
85-
String updateConfiguration(long userId, String name, String category, String value, String scope, Long id);
87+
String updateConfiguration(long userId, String name, String category, String value, ConfigKey.Scope scope, Long id);
8688

8789
// /**
8890
// * Creates a new service offering

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

Lines changed: 47 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,7 @@ public boolean stop() {
713713

714714
@Override
715715
@DB
716-
public String updateConfiguration(final long userId, final String name, final String category, String value, final String scope, final Long resourceId) {
716+
public String updateConfiguration(final long userId, final String name, final String category, String value, ConfigKey.Scope scope, final Long resourceId) {
717717
final String validationMsg = validateConfigurationValue(name, value, scope);
718718
if (validationMsg != null) {
719719
logger.error("Invalid value [{}] for configuration [{}] due to [{}].", value, name, validationMsg);
@@ -724,15 +724,14 @@ public String updateConfiguration(final long userId, final String name, final St
724724
// corresponding details table,
725725
// if scope is mentioned as global or not mentioned then it is normal
726726
// global parameter updation
727-
if (scope != null && !scope.isEmpty() && !ConfigKey.Scope.Global.toString().equalsIgnoreCase(scope)) {
727+
if (scope != null && !ConfigKey.Scope.Global.equals(scope)) {
728728
boolean valueEncrypted = shouldEncryptValue(category);
729729
if (valueEncrypted) {
730730
value = DBEncryptionUtil.encrypt(value);
731731
}
732732

733733
ApiCommandResourceType resourceType;
734-
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
735-
switch (scopeVal) {
734+
switch (scope) {
736735
case Zone:
737736
final DataCenterVO zone = _zoneDao.findById(resourceId);
738737
if (zone == null) {
@@ -831,9 +830,9 @@ public String updateConfiguration(final long userId, final String name, final St
831830

832831
CallContext.current().setEventResourceType(resourceType);
833832
CallContext.current().setEventResourceId(resourceId);
834-
CallContext.current().setEventDetails(String.format(" Name: %s, New Value: %s, Scope: %s", name, value, scope));
833+
CallContext.current().setEventDetails(String.format(" Name: %s, New Value: %s, Scope: %s", name, value, scope.name()));
835834

836-
_configDepot.invalidateConfigCache(name, scopeVal, resourceId);
835+
_configDepot.invalidateConfigCache(name, scope, resourceId);
837836
return valueEncrypted ? DBEncryptionUtil.decrypt(value) : value;
838837
}
839838

@@ -999,40 +998,40 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
999998
return _configDao.findByName(name);
1000999
}
10011000

1002-
String scope = null;
1001+
ConfigKey.Scope scope = null;
10031002
Long id = null;
10041003
int paramCountCheck = 0;
10051004

10061005
if (zoneId != null) {
1007-
scope = ConfigKey.Scope.Zone.toString();
1006+
scope = ConfigKey.Scope.Zone;
10081007
id = zoneId;
10091008
paramCountCheck++;
10101009
}
10111010
if (clusterId != null) {
1012-
scope = ConfigKey.Scope.Cluster.toString();
1011+
scope = ConfigKey.Scope.Cluster;
10131012
id = clusterId;
10141013
paramCountCheck++;
10151014
}
10161015
if (accountId != null) {
10171016
Account account = _accountMgr.getAccount(accountId);
10181017
_accountMgr.checkAccess(caller, null, false, account);
1019-
scope = ConfigKey.Scope.Account.toString();
1018+
scope = ConfigKey.Scope.Account;
10201019
id = accountId;
10211020
paramCountCheck++;
10221021
}
10231022
if (domainId != null) {
10241023
_accountMgr.checkAccess(caller, _domainDao.findById(domainId));
1025-
scope = ConfigKey.Scope.Domain.toString();
1024+
scope = ConfigKey.Scope.Domain;
10261025
id = domainId;
10271026
paramCountCheck++;
10281027
}
10291028
if (storagepoolId != null) {
1030-
scope = ConfigKey.Scope.StoragePool.toString();
1029+
scope = ConfigKey.Scope.StoragePool;
10311030
id = storagepoolId;
10321031
paramCountCheck++;
10331032
}
10341033
if (imageStoreId != null) {
1035-
scope = ConfigKey.Scope.ImageStore.toString();
1034+
scope = ConfigKey.Scope.ImageStore;
10361035
id = imageStoreId;
10371036
paramCountCheck++;
10381037
}
@@ -1046,8 +1045,15 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
10461045
if (value.isEmpty() || value.equals("null")) {
10471046
value = (id == null) ? null : "";
10481047
}
1048+
1049+
String currentValueInScope = getConfigurationValueInScope(config, name, scope, id);
10491050
final String updatedValue = updateConfiguration(userId, name, category, value, scope, id);
10501051
if (value == null && updatedValue == null || updatedValue.equalsIgnoreCase(value)) {
1052+
logger.debug("Config: {} value is updated from: {} to {} for scope: {}", name,
1053+
encryptEventValueIfConfigIsEncrypted(config, currentValueInScope),
1054+
encryptEventValueIfConfigIsEncrypted(config, value),
1055+
scope != null ? scope : ConfigKey.Scope.Global.name());
1056+
10511057
return _configDao.findByName(name);
10521058
} else {
10531059
throw new CloudRuntimeException("Unable to update configuration parameter " + name);
@@ -1109,7 +1115,7 @@ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) thr
11091115
configScope = config.getScopes();
11101116
}
11111117

1112-
String scope = "";
1118+
String scopeVal = "";
11131119
Map<String, Long> scopeMap = new LinkedHashMap<>();
11141120

11151121
Long id = null;
@@ -1125,22 +1131,23 @@ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) thr
11251131
ParamCountPair paramCountPair = getParamCount(scopeMap);
11261132
id = paramCountPair.getId();
11271133
paramCountCheck = paramCountPair.getParamCount();
1128-
scope = paramCountPair.getScope();
1134+
scopeVal = paramCountPair.getScope();
11291135

11301136
if (paramCountCheck > 1) {
11311137
throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
11321138
}
11331139

1134-
if (scope != null) {
1135-
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
1136-
if (!scope.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scopeVal)) {
1140+
if (scopeVal != null) {
1141+
ConfigKey.Scope scope = ConfigKey.Scope.valueOf(scopeVal);
1142+
if (!scopeVal.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scope)) {
11371143
throw new InvalidParameterValueException("Invalid scope id provided for the parameter " + name);
11381144
}
11391145
}
11401146

11411147
String newValue = null;
1142-
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
1143-
switch (scopeVal) {
1148+
ConfigKey.Scope scope = ConfigKey.Scope.valueOf(scopeVal);
1149+
String currentValueInScope = getConfigurationValueInScope(config, name, scope, id);
1150+
switch (scope) {
11441151
case Zone:
11451152
final DataCenterVO zone = _zoneDao.findById(id);
11461153
if (zone == null) {
@@ -1225,20 +1232,36 @@ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) thr
12251232
newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
12261233
}
12271234

1228-
_configDepot.invalidateConfigCache(name, scopeVal, id);
1235+
logger.debug("Config: {} value is updated from: {} to {} for scope: {}", name,
1236+
encryptEventValueIfConfigIsEncrypted(config, currentValueInScope),
1237+
encryptEventValueIfConfigIsEncrypted(config, newValue), scope);
1238+
1239+
_configDepot.invalidateConfigCache(name, scope, id);
12291240

12301241
CallContext.current().setEventDetails(" Name: " + name + " New Value: " + (name.toLowerCase().contains("password") ? "*****" : defaultValue == null ? "" : defaultValue));
12311242
return new Pair<Configuration, String>(_configDao.findByName(name), newValue);
12321243
}
12331244

1245+
private String getConfigurationValueInScope(ConfigurationVO config, String name, ConfigKey.Scope scope, Long id) {
1246+
String configValue;
1247+
if (scope == null || ConfigKey.Scope.Global.equals(scope)) {
1248+
configValue = config.getValue();
1249+
} else {
1250+
ConfigKey<?> configKey = _configDepot.get(name);
1251+
Object currentValue = configKey.valueInScope(scope, id);
1252+
configValue = currentValue != null ? currentValue.toString() : null;
1253+
}
1254+
return configValue;
1255+
}
1256+
12341257
/**
12351258
* Validates whether a value is valid for the specified configuration. This includes type and range validation.
12361259
* @param name name of the configuration.
12371260
* @param value value to validate.
12381261
* @param scope scope of the configuration.
12391262
* @return null if the value is valid; otherwise, returns an error message.
12401263
*/
1241-
protected String validateConfigurationValue(String name, String value, String scope) {
1264+
protected String validateConfigurationValue(String name, String value, ConfigKey.Scope scope) {
12421265
final ConfigurationVO cfg = _configDao.findByName(name);
12431266
if (cfg == null) {
12441267
logger.error("Missing configuration variable " + name + " in configuration table");
@@ -1248,10 +1271,9 @@ protected String validateConfigurationValue(String name, String value, String sc
12481271

12491272
List<ConfigKey.Scope> configScope = cfg.getScopes();
12501273
if (scope != null) {
1251-
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
1252-
if (!configScope.contains(scopeVal) &&
1274+
if (!configScope.contains(scope) &&
12531275
!(ENABLE_ACCOUNT_SETTINGS_FOR_DOMAIN.value() && configScope.contains(ConfigKey.Scope.Account) &&
1254-
scope.equals(ConfigKey.Scope.Domain.toString()))) {
1276+
ConfigKey.Scope.Domain.equals(scope))) {
12551277
logger.error("Invalid scope id provided for the parameter " + name);
12561278
return "Invalid scope id provided for the parameter " + name;
12571279
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ public void testCreateNetworkOfferingForNsx() {
459459
@Test
460460
public void testValidateInvalidConfiguration() {
461461
Mockito.doReturn(null).when(configDao).findByName(Mockito.anyString());
462-
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Global.toString());
462+
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Global);
463463
Assert.assertEquals("Invalid configuration variable.", msg);
464464
}
465465

@@ -468,7 +468,7 @@ public void testValidateInvalidScopeForConfiguration() {
468468
ConfigurationVO cfg = mock(ConfigurationVO.class);
469469
when(cfg.getScopes()).thenReturn(List.of(ConfigKey.Scope.Account));
470470
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
471-
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Domain.toString());
471+
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Domain);
472472
Assert.assertEquals("Invalid scope id provided for the parameter test.config.name", msg);
473473
}
474474

@@ -480,7 +480,7 @@ public void testValidateConfig_ThreadsOnKVMHostToTransferVMwareVMFiles_Failure()
480480
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
481481
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
482482

483-
String result = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "11", configKey.getScopes().get(0).name());
483+
String result = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "11", configKey.getScopes().get(0));
484484

485485
Assert.assertNotNull(result);
486486
}
@@ -492,7 +492,7 @@ public void testValidateConfig_ThreadsOnKVMHostToTransferVMwareVMFiles_Success()
492492
ConfigKey<Integer> configKey = UnmanagedVMsManager.ThreadsOnKVMHostToImportVMwareVMFiles;
493493
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
494494
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
495-
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "10", configKey.getScopes().get(0).name());
495+
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "10", configKey.getScopes().get(0));
496496
Assert.assertNull(msg);
497497
}
498498

@@ -505,7 +505,7 @@ public void testValidateConfig_ConvertVmwareInstanceToKvmTimeout_Failure() {
505505
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
506506
configurationManagerImplSpy.populateConfigValuesForValidationSet();
507507

508-
String result = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "0", configKey.getScopes().get(0).name());
508+
String result = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "0", configKey.getScopes().get(0));
509509

510510
Assert.assertNotNull(result);
511511
}
@@ -518,7 +518,7 @@ public void testValidateConfig_ConvertVmwareInstanceToKvmTimeout_Success() {
518518
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
519519
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
520520
configurationManagerImplSpy.populateConfigValuesForValidationSet();
521-
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "9", configKey.getScopes().get(0).name());
521+
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "9", configKey.getScopes().get(0));
522522
Assert.assertNull(msg);
523523
}
524524

@@ -633,14 +633,14 @@ public void checkIfDomainIsChildDomainTestNonChildDomainThrowException() {
633633
@Test
634634
public void validateConfigurationValueTestValidatesValueType() {
635635
Mockito.when(configKeyMock.type()).thenReturn(Integer.class);
636-
configurationManagerImplSpy.validateConfigurationValue("validate.type", "100", ConfigKey.Scope.Global.name());
636+
configurationManagerImplSpy.validateConfigurationValue("validate.type", "100", ConfigKey.Scope.Global);
637637
Mockito.verify(configurationManagerImplSpy).validateValueType("100", Integer.class);
638638
}
639639

640640
@Test
641641
public void validateConfigurationValueTestValidatesValueRange() {
642642
Mockito.when(configKeyMock.type()).thenReturn(Integer.class);
643-
configurationManagerImplSpy.validateConfigurationValue("validate.range", "100", ConfigKey.Scope.Global.name());
643+
configurationManagerImplSpy.validateConfigurationValue("validate.range", "100", ConfigKey.Scope.Global);
644644
Mockito.verify(configurationManagerImplSpy).validateValueRange("validate.range", "100", Integer.class, null);
645645
}
646646

‎server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
import org.apache.cloudstack.api.command.admin.zone.UpdateZoneCmd;
8383
import org.apache.cloudstack.api.command.user.network.ListNetworkOfferingsCmd;
8484
import org.apache.cloudstack.config.Configuration;
85+
import org.apache.cloudstack.framework.config.ConfigKey;
8586
import org.apache.cloudstack.framework.config.impl.ConfigurationSubGroupVO;
8687
import org.apache.cloudstack.region.PortableIp;
8788
import org.apache.cloudstack.region.PortableIpRange;
@@ -497,7 +498,7 @@ public String getName() {
497498
* @see com.cloud.configuration.ConfigurationManager#updateConfiguration(long, java.lang.String, java.lang.String, java.lang.String)
498499
*/
499500
@Override
500-
public String updateConfiguration(long userId, String name, String category, String value, String scope, Long resourceId) {
501+
public String updateConfiguration(long userId, String name, String category, String value, ConfigKey.Scope scope, Long resourceId) {
501502
// TODO Auto-generated method stub
502503
return null;
503504
}

0 commit comments

Comments
 (0)