diff --git a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java index 1694b19c33fd..4fbebf71d4c6 100644 --- a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java +++ b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java @@ -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 diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 56a86e65da02..11c0f9d8181b 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -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); @@ -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) { @@ -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; } @@ -979,40 +978,40 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP return _configDao.findByName(name); } - String scope = null; + ConfigKey.Scope scope = null; 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++; } @@ -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); @@ -1089,7 +1095,7 @@ public Pair resetConfiguration(final ResetCfgCmd cmd) thr configScope = config.getScopes(); } - String scope = ""; + String scopeVal = ""; Map scopeMap = new LinkedHashMap<>(); Long id = null; @@ -1105,22 +1111,23 @@ public Pair 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) { @@ -1205,12 +1212,28 @@ public Pair 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(_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. @@ -1218,7 +1241,7 @@ public Pair resetConfiguration(final ResetCfgCmd cmd) thr * @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"); @@ -1227,10 +1250,9 @@ protected String validateConfigurationValue(String name, String value, String sc List 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; } diff --git a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java index 8c714b57cdbb..ccd3843df94c 100644 --- a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java +++ b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java @@ -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); } @@ -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); } @@ -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); } @@ -488,7 +488,7 @@ public void testValidateConfig_ThreadsOnKVMHostToTransferVMwareVMFiles_Success() ConfigKey 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); } @@ -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); } @@ -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); } @@ -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); } diff --git a/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java index cdd23b0ccc2c..d6d746c1a367 100644 --- a/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java @@ -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; @@ -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; }