Skip to content

Log previous and new value of configuration when reset/update API is called #10769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaanHoogland merged 4 commits into from
Jun 4, 2025
Merged

Log previous and new value of configuration when reset/update API is called #10769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7280d5b
Log previous and new value of configuration before reset/update
sudo87 Apr 22, 2025
74a00c4
Following changes are done in this commit:
sudo87 Apr 24, 2025
9a7de41
Merge branch 'main' into logConfigUpdate
sudo87 May 15, 2025
fd401eb
Fix updateCfg for null scope, it is considered Global
sudo87 May 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,14 @@ public interface ConfigurationManager {

/**
* Updates a configuration entry with a new value
*
* @param userId
* @param name
* @param category
* @param value
* @param scope
* @param id
*/
String updateConfiguration(long userId, String name, String category, String value, String scope, Long id);
String updateConfiguration(long userId, String name, String category, String value, ConfigKey.Scope scope, Long id);

// /**
// * Creates a new service offering
Expand Down
72 changes: 47 additions & 25 deletions server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ public boolean stop() {

@Override
@DB
public String updateConfiguration(final long userId, final String name, final String category, String value, final String scope, final Long resourceId) {
public String updateConfiguration(final long userId, final String name, final String category, String value, ConfigKey.Scope scope, final Long resourceId) {
final String validationMsg = validateConfigurationValue(name, value, scope);
if (validationMsg != null) {
logger.error("Invalid value [{}] for configuration [{}] due to [{}].", value, name, validationMsg);
Expand All @@ -704,15 +704,14 @@ public String updateConfiguration(final long userId, final String name, final St
// corresponding details table,
// if scope is mentioned as global or not mentioned then it is normal
// global parameter updation
if (scope != null && !scope.isEmpty() && !ConfigKey.Scope.Global.toString().equalsIgnoreCase(scope)) {
if (scope != null && !ConfigKey.Scope.Global.equals(scope)) {
boolean valueEncrypted = shouldEncryptValue(category);
if (valueEncrypted) {
value = DBEncryptionUtil.encrypt(value);
}

ApiCommandResourceType resourceType;
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
switch (scopeVal) {
switch (scope) {
case Zone:
final DataCenterVO zone = _zoneDao.findById(resourceId);
if (zone == null) {
Expand Down Expand Up @@ -811,9 +810,9 @@ public String updateConfiguration(final long userId, final String name, final St

CallContext.current().setEventResourceType(resourceType);
CallContext.current().setEventResourceId(resourceId);
CallContext.current().setEventDetails(String.format(" Name: %s, New Value: %s, Scope: %s", name, value, scope));
CallContext.current().setEventDetails(String.format(" Name: %s, New Value: %s, Scope: %s", name, value, scope.name()));

_configDepot.invalidateConfigCache(name, scopeVal, resourceId);
_configDepot.invalidateConfigCache(name, scope, resourceId);
return valueEncrypted ? DBEncryptionUtil.decrypt(value) : value;
}

Expand Down Expand Up @@ -979,40 +978,40 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
return _configDao.findByName(name);
}

String scope = null;
ConfigKey.Scope scope = null;
Comment thread
sudo87 marked this conversation as resolved.
Long id = null;
int paramCountCheck = 0;

if (zoneId != null) {
scope = ConfigKey.Scope.Zone.toString();
scope = ConfigKey.Scope.Zone;
id = zoneId;
paramCountCheck++;
}
if (clusterId != null) {
scope = ConfigKey.Scope.Cluster.toString();
scope = ConfigKey.Scope.Cluster;
id = clusterId;
paramCountCheck++;
}
if (accountId != null) {
Account account = _accountMgr.getAccount(accountId);
_accountMgr.checkAccess(caller, null, false, account);
scope = ConfigKey.Scope.Account.toString();
scope = ConfigKey.Scope.Account;
id = accountId;
paramCountCheck++;
}
if (domainId != null) {
_accountMgr.checkAccess(caller, _domainDao.findById(domainId));
scope = ConfigKey.Scope.Domain.toString();
scope = ConfigKey.Scope.Domain;
id = domainId;
paramCountCheck++;
}
if (storagepoolId != null) {
scope = ConfigKey.Scope.StoragePool.toString();
scope = ConfigKey.Scope.StoragePool;
id = storagepoolId;
paramCountCheck++;
}
if (imageStoreId != null) {
scope = ConfigKey.Scope.ImageStore.toString();
scope = ConfigKey.Scope.ImageStore;
id = imageStoreId;
paramCountCheck++;
}
Expand All @@ -1026,8 +1025,15 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
if (value.isEmpty() || value.equals("null")) {
value = (id == null) ? null : "";
}

String currentValueInScope = getConfigurationValueInScope(config, name, scope, id);
final String updatedValue = updateConfiguration(userId, name, category, value, scope, id);
if (value == null && updatedValue == null || updatedValue.equalsIgnoreCase(value)) {
logger.debug("Config: {} value is updated from: {} to {} for scope: {}", name,
encryptEventValueIfConfigIsEncrypted(config, currentValueInScope),
encryptEventValueIfConfigIsEncrypted(config, value),
scope != null ? scope : ConfigKey.Scope.Global.name());

return _configDao.findByName(name);
} else {
throw new CloudRuntimeException("Unable to update configuration parameter " + name);
Expand Down Expand Up @@ -1089,7 +1095,7 @@ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) thr
configScope = config.getScopes();
}

String scope = "";
String scopeVal = "";
Map<String, Long> scopeMap = new LinkedHashMap<>();

Long id = null;
Expand All @@ -1105,22 +1111,23 @@ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) thr
ParamCountPair paramCountPair = getParamCount(scopeMap);
id = paramCountPair.getId();
paramCountCheck = paramCountPair.getParamCount();
scope = paramCountPair.getScope();
scopeVal = paramCountPair.getScope();

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

if (scope != null) {
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
if (!scope.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scopeVal)) {
if (scopeVal != null) {
ConfigKey.Scope scope = ConfigKey.Scope.valueOf(scopeVal);
if (!scopeVal.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scope)) {
throw new InvalidParameterValueException("Invalid scope id provided for the parameter " + name);
}
}

String newValue = null;
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
switch (scopeVal) {
ConfigKey.Scope scope = ConfigKey.Scope.valueOf(scopeVal);
String currentValueInScope = getConfigurationValueInScope(config, name, scope, id);
switch (scope) {
case Zone:
final DataCenterVO zone = _zoneDao.findById(id);
if (zone == null) {
Expand Down Expand Up @@ -1205,20 +1212,36 @@ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) thr
newValue = optionalValue.isPresent() ? optionalValue.get().toString() : defaultValue;
}

_configDepot.invalidateConfigCache(name, scopeVal, id);
logger.debug("Config: {} value is updated from: {} to {} for scope: {}", name,
encryptEventValueIfConfigIsEncrypted(config, currentValueInScope),
encryptEventValueIfConfigIsEncrypted(config, newValue), scope);

_configDepot.invalidateConfigCache(name, scope, id);

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

private String getConfigurationValueInScope(ConfigurationVO config, String name, ConfigKey.Scope scope, Long id) {
String configValue;
if (scope == null || ConfigKey.Scope.Global.equals(scope)) {
configValue = config.getValue();
} else {
ConfigKey<?> configKey = _configDepot.get(name);
Object currentValue = configKey.valueInScope(scope, id);
configValue = currentValue != null ? currentValue.toString() : null;
}
return configValue;
}

/**
* Validates whether a value is valid for the specified configuration. This includes type and range validation.
* @param name name of the configuration.
* @param value value to validate.
* @param scope scope of the configuration.
* @return null if the value is valid; otherwise, returns an error message.
*/
protected String validateConfigurationValue(String name, String value, String scope) {
protected String validateConfigurationValue(String name, String value, ConfigKey.Scope scope) {
final ConfigurationVO cfg = _configDao.findByName(name);
if (cfg == null) {
logger.error("Missing configuration variable " + name + " in configuration table");
Expand All @@ -1227,10 +1250,9 @@ protected String validateConfigurationValue(String name, String value, String sc

List<ConfigKey.Scope> configScope = cfg.getScopes();
if (scope != null) {
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
if (!configScope.contains(scopeVal) &&
if (!configScope.contains(scope) &&
!(ENABLE_ACCOUNT_SETTINGS_FOR_DOMAIN.value() && configScope.contains(ConfigKey.Scope.Account) &&
scope.equals(ConfigKey.Scope.Domain.toString()))) {
ConfigKey.Scope.Domain.equals(scope))) {
logger.error("Invalid scope id provided for the parameter " + name);
return "Invalid scope id provided for the parameter " + name;
}
Expand Down
16 changes: 8 additions & 8 deletions server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ public void testCreateNetworkOfferingForNsx() {
@Test
public void testValidateInvalidConfiguration() {
Mockito.doReturn(null).when(configDao).findByName(Mockito.anyString());
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Global.toString());
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Global);
Assert.assertEquals("Invalid configuration variable.", msg);
}

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

Expand All @@ -476,7 +476,7 @@ public void testValidateConfig_ThreadsOnKVMHostToTransferVMwareVMFiles_Failure()
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());

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

Assert.assertNotNull(result);
}
Expand All @@ -488,7 +488,7 @@ public void testValidateConfig_ThreadsOnKVMHostToTransferVMwareVMFiles_Success()
ConfigKey<Integer> configKey = UnmanagedVMsManager.ThreadsOnKVMHostToImportVMwareVMFiles;
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "10", configKey.getScopes().get(0).name());
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "10", configKey.getScopes().get(0));
Assert.assertNull(msg);
}

Expand All @@ -501,7 +501,7 @@ public void testValidateConfig_ConvertVmwareInstanceToKvmTimeout_Failure() {
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
configurationManagerImplSpy.populateConfigValuesForValidationSet();

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

Assert.assertNotNull(result);
}
Expand All @@ -514,7 +514,7 @@ public void testValidateConfig_ConvertVmwareInstanceToKvmTimeout_Success() {
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
configurationManagerImplSpy.populateConfigValuesForValidationSet();
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "9", configKey.getScopes().get(0).name());
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "9", configKey.getScopes().get(0));
Assert.assertNull(msg);
}

Expand Down Expand Up @@ -629,14 +629,14 @@ public void checkIfDomainIsChildDomainTestNonChildDomainThrowException() {
@Test
public void validateConfigurationValueTestValidatesValueType() {
Mockito.when(configKeyMock.type()).thenReturn(Integer.class);
configurationManagerImplSpy.validateConfigurationValue("validate.type", "100", ConfigKey.Scope.Global.name());
configurationManagerImplSpy.validateConfigurationValue("validate.type", "100", ConfigKey.Scope.Global);
Mockito.verify(configurationManagerImplSpy).validateValueType("100", Integer.class);
}

@Test
public void validateConfigurationValueTestValidatesValueRange() {
Mockito.when(configKeyMock.type()).thenReturn(Integer.class);
configurationManagerImplSpy.validateConfigurationValue("validate.range", "100", ConfigKey.Scope.Global.name());
configurationManagerImplSpy.validateConfigurationValue("validate.range", "100", ConfigKey.Scope.Global);
Mockito.verify(configurationManagerImplSpy).validateValueRange("validate.range", "100", Integer.class, null);
}

Expand Down
3 changes: 2 additions & 1 deletion server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
import org.apache.cloudstack.api.command.admin.zone.UpdateZoneCmd;
import org.apache.cloudstack.api.command.user.network.ListNetworkOfferingsCmd;
import org.apache.cloudstack.config.Configuration;
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.framework.config.impl.ConfigurationSubGroupVO;
import org.apache.cloudstack.region.PortableIp;
import org.apache.cloudstack.region.PortableIpRange;
Expand Down Expand Up @@ -497,7 +498,7 @@ public String getName() {
* @see com.cloud.configuration.ConfigurationManager#updateConfiguration(long, java.lang.String, java.lang.String, java.lang.String)
*/
@Override
public String updateConfiguration(long userId, String name, String category, String value, String scope, Long resourceId) {
public String updateConfiguration(long userId, String name, String category, String value, ConfigKey.Scope scope, Long resourceId) {
// TODO Auto-generated method stub
return null;
}
Expand Down