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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

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
2 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
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 @@

@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) {

Check warning on line 696 in server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L696 

Added line #L696 was not covered by tests
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 @@
// 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 @@

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()));

Check warning on line 813 in server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L813 

Added line #L813 was not covered by tests

_configDepot.invalidateConfigCache(name, scopeVal, resourceId);
_configDepot.invalidateConfigCache(name, scope, resourceId);

Check warning on line 815 in server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L815 

Added line #L815 was not covered by tests
return valueEncrypted ? DBEncryptionUtil.decrypt(value) : value;
}

Expand Down Expand Up @@ -979,40 +978,40 @@
return _configDao.findByName(name);
}

String scope = null;
ConfigKey.Scope scope = ConfigKey.Scope.Global;

Check warning on line 981 in server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L981 

Added line #L981 was not covered by tests
Long id = null;
int paramCountCheck = 0;

if (zoneId != null) {
scope = ConfigKey.Scope.Zone.toString();
scope = ConfigKey.Scope.Zone;

Check warning on line 986 in server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L986 

Added line #L986 was not covered by tests
id = zoneId;
paramCountCheck++;
}
if (clusterId != null) {
scope = ConfigKey.Scope.Cluster.toString();
scope = ConfigKey.Scope.Cluster;

Check warning on line 991 in server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L991 

Added line #L991 was not covered by tests
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;

Check warning on line 998 in server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L998 

Added line #L998 was not covered by tests
id = accountId;
paramCountCheck++;
}
if (domainId != null) {
_accountMgr.checkAccess(caller, _domainDao.findById(domainId));
scope = ConfigKey.Scope.Domain.toString();
scope = ConfigKey.Scope.Domain;

Check warning on line 1004 in server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L1004 

Added line #L1004 was not covered by tests
id = domainId;
paramCountCheck++;
}
if (storagepoolId != null) {
scope = ConfigKey.Scope.StoragePool.toString();
scope = ConfigKey.Scope.StoragePool;

Check warning on line 1009 in server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L1009 

Added line #L1009 was not covered by tests
id = storagepoolId;
paramCountCheck++;
}
if (imageStoreId != null) {
scope = ConfigKey.Scope.ImageStore.toString();
scope = ConfigKey.Scope.ImageStore;

Check warning on line 1014 in server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L1014 

Added line #L1014 was not covered by tests
id = imageStoreId;
paramCountCheck++;
}
Expand All @@ -1026,8 +1025,15 @@
if (value.isEmpty() || value.equals("null")) {
value = (id == null) ? null : "";
}

String currentValueInScope = getConfigurationValueInScope(config, name, scope, id);

Check warning on line 1029 in server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L1029 

Added line #L1029 was not covered by tests
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),

Check warning on line 1034 in server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L1032-L1034 

Added lines #L1032 - L1034 were not covered by tests
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 @@
configScope = config.getScopes();
}

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

Long id = null;
Expand All @@ -1105,22 +1111,23 @@
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 @@
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 (ConfigKey.Scope.Global.equals(scope)) {
configValue = config.getValue();

Check warning on line 1228 in server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L1228 

Added line #L1228 was not covered by tests
} 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 @@

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
Loading