Skip to content

Commit 1984642

Browse files
sudo87dhslove
sudo87
authored and
dhslove
committed
Log previous and new value of configuration when reset/update API is called (apache#10769)
1 parent 33bd1b9 commit 1984642

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
@@ -727,7 +727,7 @@ public boolean stop() {
727727

728728
@Override
729729
@DB
730-
public String updateConfiguration(final long userId, final String name, final String category, String value, final String scope, final Long resourceId) {
730+
public String updateConfiguration(final long userId, final String name, final String category, String value, ConfigKey.Scope scope, final Long resourceId) {
731731
final String validationMsg = validateConfigurationValue(name, value, scope);
732732
if (validationMsg != null) {
733733
logger.error("Invalid value [{}] for configuration [{}] due to [{}].", value, name, validationMsg);
@@ -738,15 +738,14 @@ public String updateConfiguration(final long userId, final String name, final St
738738
// corresponding details table,
739739
// if scope is mentioned as global or not mentioned then it is normal
740740
// global parameter updation
741-
if (scope != null && !scope.isEmpty() && !ConfigKey.Scope.Global.toString().equalsIgnoreCase(scope)) {
741+
if (scope != null && !ConfigKey.Scope.Global.equals(scope)) {
742742
boolean valueEncrypted = shouldEncryptValue(category);
743743
if (valueEncrypted) {
744744
value = DBEncryptionUtil.encrypt(value);
745745
}
746746

747747
ApiCommandResourceType resourceType;
748-
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
749-
switch (scopeVal) {
748+
switch (scope) {
750749
case Zone:
751750
final DataCenterVO zone = _zoneDao.findById(resourceId);
752751
if (zone == null) {
@@ -845,9 +844,9 @@ public String updateConfiguration(final long userId, final String name, final St
845844

846845
CallContext.current().setEventResourceType(resourceType);
847846
CallContext.current().setEventResourceId(resourceId);
848-
CallContext.current().setEventDetails(String.format(" Name: %s, New Value: %s, Scope: %s", name, value, scope));
847+
CallContext.current().setEventDetails(String.format(" Name: %s, New Value: %s, Scope: %s", name, value, scope.name()));
849848

850-
_configDepot.invalidateConfigCache(name, scopeVal, resourceId);
849+
_configDepot.invalidateConfigCache(name, scope, resourceId);
851850
return valueEncrypted ? DBEncryptionUtil.decrypt(value) : value;
852851
}
853852

@@ -1013,40 +1012,40 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
10131012
return _configDao.findByName(name);
10141013
}
10151014

1016-
String scope = null;
1015+
ConfigKey.Scope scope = null;
10171016
Long id = null;
10181017
int paramCountCheck = 0;
10191018

10201019
if (zoneId != null) {
1021-
scope = ConfigKey.Scope.Zone.toString();
1020+
scope = ConfigKey.Scope.Zone;
10221021
id = zoneId;
10231022
paramCountCheck++;
10241023
}
10251024
if (clusterId != null) {
1026-
scope = ConfigKey.Scope.Cluster.toString();
1025+
scope = ConfigKey.Scope.Cluster;
10271026
id = clusterId;
10281027
paramCountCheck++;
10291028
}
10301029
if (accountId != null) {
10311030
Account account = _accountMgr.getAccount(accountId);
10321031
_accountMgr.checkAccess(caller, null, false, account);
1033-
scope = ConfigKey.Scope.Account.toString();
1032+
scope = ConfigKey.Scope.Account;
10341033
id = accountId;
10351034
paramCountCheck++;
10361035
}
10371036
if (domainId != null) {
10381037
_accountMgr.checkAccess(caller, _domainDao.findById(domainId));
1039-
scope = ConfigKey.Scope.Domain.toString();
1038+
scope = ConfigKey.Scope.Domain;
10401039
id = domainId;
10411040
paramCountCheck++;
10421041
}
10431042
if (storagepoolId != null) {
1044-
scope = ConfigKey.Scope.StoragePool.toString();
1043+
scope = ConfigKey.Scope.StoragePool;
10451044
id = storagepoolId;
10461045
paramCountCheck++;
10471046
}
10481047
if (imageStoreId != null) {
1049-
scope = ConfigKey.Scope.ImageStore.toString();
1048+
scope = ConfigKey.Scope.ImageStore;
10501049
id = imageStoreId;
10511050
paramCountCheck++;
10521051
}
@@ -1060,8 +1059,15 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
10601059
if (value.isEmpty() || value.equals("null")) {
10611060
value = (id == null) ? null : "";
10621061
}
1062+
1063+
String currentValueInScope = getConfigurationValueInScope(config, name, scope, id);
10631064
final String updatedValue = updateConfiguration(userId, name, category, value, scope, id);
10641065
if (value == null && updatedValue == null || updatedValue.equalsIgnoreCase(value)) {
1066+
logger.debug("Config: {} value is updated from: {} to {} for scope: {}", name,
1067+
encryptEventValueIfConfigIsEncrypted(config, currentValueInScope),
1068+
encryptEventValueIfConfigIsEncrypted(config, value),
1069+
scope != null ? scope : ConfigKey.Scope.Global.name());
1070+
10651071
return _configDao.findByName(name);
10661072
} else {
10671073
throw new CloudRuntimeException("Unable to update configuration parameter " + name);
@@ -1123,7 +1129,7 @@ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) thr
11231129
configScope = config.getScopes();
11241130
}
11251131

1126-
String scope = "";
1132+
String scopeVal = "";
11271133
Map<String, Long> scopeMap = new LinkedHashMap<>();
11281134

11291135
Long id = null;
@@ -1139,22 +1145,23 @@ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) thr
11391145
ParamCountPair paramCountPair = getParamCount(scopeMap);
11401146
id = paramCountPair.getId();
11411147
paramCountCheck = paramCountPair.getParamCount();
1142-
scope = paramCountPair.getScope();
1148+
scopeVal = paramCountPair.getScope();
11431149

11441150
if (paramCountCheck > 1) {
11451151
throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
11461152
}
11471153

1148-
if (scope != null) {
1149-
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
1150-
if (!scope.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scopeVal)) {
1154+
if (scopeVal != null) {
1155+
ConfigKey.Scope scope = ConfigKey.Scope.valueOf(scopeVal);
1156+
if (!scopeVal.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scope)) {
11511157
throw new InvalidParameterValueException("Invalid scope id provided for the parameter " + name);
11521158
}
11531159
}
11541160

11551161
String newValue = null;
1156-
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
1157-
switch (scopeVal) {
1162+
ConfigKey.Scope scope = ConfigKey.Scope.valueOf(scopeVal);
1163+
String currentValueInScope = getConfigurationValueInScope(config, name, scope, id);
1164+
switch (scope) {
11581165
case Zone:
11591166
final DataCenterVO zone = _zoneDao.findById(id);
11601167
if (zone == null) {
@@ -1239,20 +1246,36 @@ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) thr
12391246
newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
12401247
}
12411248

1242-
_configDepot.invalidateConfigCache(name, scopeVal, id);
1249+
logger.debug("Config: {} value is updated from: {} to {} for scope: {}", name,
1250+
encryptEventValueIfConfigIsEncrypted(config, currentValueInScope),
1251+
encryptEventValueIfConfigIsEncrypted(config, newValue), scope);
1252+
1253+
_configDepot.invalidateConfigCache(name, scope, id);
12431254

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

1259+
private String getConfigurationValueInScope(ConfigurationVO config, String name, ConfigKey.Scope scope, Long id) {
1260+
String configValue;
1261+
if (scope == null || ConfigKey.Scope.Global.equals(scope)) {
1262+
configValue = config.getValue();
1263+
} else {
1264+
ConfigKey<?> configKey = _configDepot.get(name);
1265+
Object currentValue = configKey.valueInScope(scope, id);
1266+
configValue = currentValue != null ? currentValue.toString() : null;
1267+
}
1268+
return configValue;
1269+
}
1270+
12481271
/**
12491272
* Validates whether a value is valid for the specified configuration. This includes type and range validation.
12501273
* @param name name of the configuration.
12511274
* @param value value to validate.
12521275
* @param scope scope of the configuration.
12531276
* @return null if the value is valid; otherwise, returns an error message.
12541277
*/
1255-
protected String validateConfigurationValue(String name, String value, String scope) {
1278+
protected String validateConfigurationValue(String name, String value, ConfigKey.Scope scope) {
12561279
final ConfigurationVO cfg = _configDao.findByName(name);
12571280
if (cfg == null) {
12581281
logger.error("Missing configuration variable " + name + " in configuration table");
@@ -1262,10 +1285,9 @@ protected String validateConfigurationValue(String name, String value, String sc
12621285

12631286
List<ConfigKey.Scope> configScope = cfg.getScopes();
12641287
if (scope != null) {
1265-
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
1266-
if (!configScope.contains(scopeVal) &&
1288+
if (!configScope.contains(scope) &&
12671289
!(ENABLE_ACCOUNT_SETTINGS_FOR_DOMAIN.value() && configScope.contains(ConfigKey.Scope.Account) &&
1268-
scope.equals(ConfigKey.Scope.Domain.toString()))) {
1290+
ConfigKey.Scope.Domain.equals(scope))) {
12691291
logger.error("Invalid scope id provided for the parameter " + name);
12701292
return "Invalid scope id provided for the parameter " + name;
12711293
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ public void testCreateNetworkOfferingForNsx() {
467467
@Test
468468
public void testValidateInvalidConfiguration() {
469469
Mockito.doReturn(null).when(configDao).findByName(Mockito.anyString());
470-
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Global.toString());
470+
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Global);
471471
Assert.assertEquals("Invalid configuration variable.", msg);
472472
}
473473

@@ -476,7 +476,7 @@ public void testValidateInvalidScopeForConfiguration() {
476476
ConfigurationVO cfg = mock(ConfigurationVO.class);
477477
when(cfg.getScopes()).thenReturn(List.of(ConfigKey.Scope.Account));
478478
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
479-
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Domain.toString());
479+
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Domain);
480480
Assert.assertEquals("Invalid scope id provided for the parameter test.config.name", msg);
481481
}
482482

@@ -488,7 +488,7 @@ public void testValidateConfig_ThreadsOnKVMHostToTransferVMwareVMFiles_Failure()
488488
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
489489
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
490490

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

493493
Assert.assertNotNull(result);
494494
}
@@ -500,7 +500,7 @@ public void testValidateConfig_ThreadsOnKVMHostToTransferVMwareVMFiles_Success()
500500
ConfigKey<Integer> configKey = UnmanagedVMsManager.ThreadsOnKVMHostToImportVMwareVMFiles;
501501
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
502502
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
503-
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "10", configKey.getScopes().get(0).name());
503+
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "10", configKey.getScopes().get(0));
504504
Assert.assertNull(msg);
505505
}
506506

@@ -513,7 +513,7 @@ public void testValidateConfig_ConvertVmwareInstanceToKvmTimeout_Failure() {
513513
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
514514
configurationManagerImplSpy.populateConfigValuesForValidationSet();
515515

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

518518
Assert.assertNotNull(result);
519519
}
@@ -526,7 +526,7 @@ public void testValidateConfig_ConvertVmwareInstanceToKvmTimeout_Success() {
526526
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
527527
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
528528
configurationManagerImplSpy.populateConfigValuesForValidationSet();
529-
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "9", configKey.getScopes().get(0).name());
529+
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "9", configKey.getScopes().get(0));
530530
Assert.assertNull(msg);
531531
}
532532

@@ -641,14 +641,14 @@ public void checkIfDomainIsChildDomainTestNonChildDomainThrowException() {
641641
@Test
642642
public void validateConfigurationValueTestValidatesValueType() {
643643
Mockito.when(configKeyMock.type()).thenReturn(Integer.class);
644-
configurationManagerImplSpy.validateConfigurationValue("validate.type", "100", ConfigKey.Scope.Global.name());
644+
configurationManagerImplSpy.validateConfigurationValue("validate.type", "100", ConfigKey.Scope.Global);
645645
Mockito.verify(configurationManagerImplSpy).validateValueType("100", Integer.class);
646646
}
647647

648648
@Test
649649
public void validateConfigurationValueTestValidatesValueRange() {
650650
Mockito.when(configKeyMock.type()).thenReturn(Integer.class);
651-
configurationManagerImplSpy.validateConfigurationValue("validate.range", "100", ConfigKey.Scope.Global.name());
651+
configurationManagerImplSpy.validateConfigurationValue("validate.range", "100", ConfigKey.Scope.Global);
652652
Mockito.verify(configurationManagerImplSpy).validateValueRange("validate.range", "100", Integer.class, null);
653653
}
654654

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