Configuration validation improvements, and some code refactoring

Author: sureshanaparti

engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java

@@ -20,9 +20,17 @@
 import java.util.List;
 
 import org.apache.cloudstack.api.response.MigrationResponse;
+import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.storage.ImageStoreService.MigrationPolicy;
 
 public interface StorageOrchestrationService {
+    ConfigKey<Double> ImageStoreImbalanceThreshold = new ConfigKey<>("Advanced", Double.class,
+            "image.store.imbalance.threshold",
+            "0.3",
+            "The storage imbalance threshold that is compared with the standard deviation percentage for a storage utilization metric. " +
+                    "The value is a percentage in decimal format.",
+            true, ConfigKey.Scope.Global);
+
     MigrationResponse migrateData(Long srcDataStoreId, List<Long> destDatastores, MigrationPolicy migrationPolicy);
 
     MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreId, List<Long> templateIdList, List<Long> snapshotIdList);

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java

@@ -99,13 +99,6 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra
     @Inject
     DataMigrationUtility migrationHelper;
 
-    ConfigKey<Double> ImageStoreImbalanceThreshold = new ConfigKey<>("Advanced", Double.class,
-            "image.store.imbalance.threshold",
-            "0.3",
-            "The storage imbalance threshold that is compared with the standard deviation percentage for a storage utilization metric. " +
-                    "The value is a percentage in decimal format.",
-            true, ConfigKey.Scope.Global);
-
     Integer numConcurrentCopyTasksPerSSVM = 2;
 
     @Override

framework/quota/src/main/java/org/apache/cloudstack/quota/constant/QuotaConfig.java

@@ -24,7 +24,7 @@ public interface QuotaConfig {
     public static final ConfigKey<Boolean> QuotaPluginEnabled = new ConfigKey<Boolean>("Advanced", Boolean.class, "quota.enable.service", "false",
             "Indicates whether Quota plugin is enabled or not.", true);
 
-    public static final ConfigKey<String> QuotaEnableEnforcement = new ConfigKey<String>("Advanced", String.class, "quota.enable.enforcement", "false",
+    public static final ConfigKey<Boolean> QuotaEnableEnforcement = new ConfigKey<Boolean>("Advanced", Boolean.class, "quota.enable.enforcement", "false",
             "Enable the usage quota enforcement, i.e. on true when exceeding quota the respective account will be locked.", true);
 
     public static final ConfigKey<String> QuotaCurrencySymbol = new ConfigKey<String>("Advanced", String.class, "quota.currency.symbol", "$",
@@ -57,7 +57,7 @@ public interface QuotaConfig {
     public static final ConfigKey<String> QuotaSmtpEnabledSecurityProtocols = new ConfigKey<String>("Advanced", String.class, "quota.usage.smtp.enabledSecurityProtocols", "",
             "White-space separated security protocols; ex: \"TLSv1 TLSv1.1\". Supported protocols: SSLv2Hello, SSLv3, TLSv1, TLSv1.1 and TLSv1.2.", true);
 
-    public static final ConfigKey<String> QuotaSmtpUseStartTLS = new ConfigKey<String>("Advanced", String.class, "quota.usage.smtp.useStartTLS", "false",
+    public static final ConfigKey<Boolean> QuotaSmtpUseStartTLS = new ConfigKey<Boolean>("Advanced", Boolean.class, "quota.usage.smtp.useStartTLS", "false",
             "If set to true and if we enable security via quota.usage.smtp.useAuth, this will enable StartTLS to secure the connection.", true);
 
     public static final ConfigKey<Long> QuotaActivationRuleTimeout = new ConfigKey<>("Advanced", Long.class, "quota.activationrule.timeout", "2000", "The maximum runtime,"

plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java

@@ -504,7 +504,7 @@ public QuotaCreditsResponse addQuotaCredits(Long accountId, Long domainId, Doubl
         if (account == null) {
             throw new InvalidParameterValueException("Account does not exist with account id " + accountId);
         }
-        final boolean lockAccountEnforcement = "true".equalsIgnoreCase(QuotaConfig.QuotaEnableEnforcement.value());
+        final boolean lockAccountEnforcement = QuotaConfig.QuotaEnableEnforcement.value();
         final BigDecimal currentAccountBalance = _quotaBalanceDao.lastQuotaBalance(accountId, domainId, startOfNextDay(new Date(despositedOn.getTime())));
         if (s_logger.isDebugEnabled()) {
             s_logger.debug("AddQuotaCredits: Depositing " + amount + " on adjusted date " + despositedOn + ", current balance " + currentAccountBalance);

server/src/main/java/com/cloud/configuration/Config.java

@@ -941,7 +941,7 @@ public enum Config {
     ElasticLoadBalancerEnabled(
             "Advanced",
             ManagementServer.class,
-            String.class,
+            Boolean.class,
             "network.loadbalancer.basiczone.elb.enabled",
             "false",
             "Whether the load balancing service is enabled for basic zones",

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

@@ -100,6 +100,7 @@
 import org.apache.cloudstack.config.ApiServiceConfiguration;
 import org.apache.cloudstack.config.Configuration;
 import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
 import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
 import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
@@ -594,6 +595,8 @@ private void weightBasedParametersForValidation() {
         weightBasedParametersForValidation.add(CapacityManager.SecondaryStorageCapacityThreshold.key());
         weightBasedParametersForValidation.add(ClusterDrsService.ClusterDrsImbalanceThreshold.key());
         weightBasedParametersForValidation.add(ClusterDrsService.ClusterDrsImbalanceSkipThreshold.key());
+        weightBasedParametersForValidation.add(StorageOrchestrationService.ImageStoreImbalanceThreshold.key());
+        weightBasedParametersForValidation.add(AlertManager.Ipv6SubnetCapacityThreshold.key());
     }
 
     private void overProvisioningFactorsForValidation() {
@@ -1205,7 +1208,7 @@ protected String validateConfigurationValue(final String name, String value, fin
                 return "Invalid scope id provided for the parameter " + name;
             }
         }
-        Class<?> type = null;
+        Class<?> type;
         final Config configuration = Config.getConfig(name);
         if (configuration == null) {
             s_logger.warn("Did not find configuration " + name + " in Config.java. Perhaps moved to ConfigDepot");
@@ -1218,25 +1221,10 @@ protected String validateConfigurationValue(final String name, String value, fin
         } else {
             type = configuration.getType();
         }
+
         //no need to validate further if a
         //config can have null value.
-        String errMsg = null;
-        try {
-            if (type.equals(Integer.class)) {
-                errMsg = "There was error in trying to parse value: " + value + ". Please enter a valid integer value for parameter " + name;
-                Integer.parseInt(value);
-            } else if (type.equals(Float.class)) {
-                errMsg = "There was error in trying to parse value: " + value + ". Please enter a valid float value for parameter " + name;
-                Float.parseFloat(value);
-            } else if (type.equals(Long.class)) {
-                errMsg = "There was error in trying to parse value: " + value + ". Please enter a valid long value for parameter " + name;
-                Long.parseLong(value);
-            }
-        } catch (final Exception e) {
-            // catching generic exception as some throws NullPointerException and some throws NumberFormatExcpeion
-            s_logger.error(errMsg);
-            return errMsg;
-        }
+        String errMsg = validateConfigValueAndType(name, value, type);
 
         if (value == null) {
             if (type.equals(Boolean.class)) {
@@ -1328,6 +1316,18 @@ protected String validateConfigurationValue(final String name, String value, fin
             }
         }
 
+        if (type.equals(Double.class)) {
+            try {
+                final Double val = Double.parseDouble(value);
+                if (weightBasedParametersForValidation.contains(name) && (val < 0f || val > 1f)) {
+                    throw new InvalidParameterValueException("Please enter a value between 0 and 1 for the configuration parameter: " + name);
+                }
+            } catch (final NumberFormatException e) {
+                s_logger.error("There was an error trying to parse the double value for configuration parameter: " + name);
+                throw new InvalidParameterValueException("There was an error trying to parse the double value for configuration parameter: " + name);
+            }
+        }
+
         if (type.equals(String.class)) {
             if (name.equalsIgnoreCase(SecStorageAllowedInternalDownloadSites.key()) && StringUtils.isNotEmpty(value)) {
                 final String[] cidrs = value.split(",");
@@ -1386,6 +1386,30 @@ protected void validateConfigurationAllowedOnlyForDefaultAdmin(String configName
         }
     }
 
+    private String validateConfigValueAndType(String name, String value, Class<?> type) {
+        String errMsg = null;
+        try {
+            if (type.equals(Integer.class)) {
+                errMsg = "There was error in trying to parse value: " + value + ". Please enter a valid integer value for parameter " + name;
+                Integer.parseInt(value);
+            } else if (type.equals(Float.class)) {
+                errMsg = "There was error in trying to parse value: " + value + ". Please enter a valid float value for parameter " + name;
+                Float.parseFloat(value);
+            } else if (type.equals(Long.class)) {
+                errMsg = "There was error in trying to parse value: " + value + ". Please enter a valid long value for parameter " + name;
+                Long.parseLong(value);
+            } else if (type.equals(Double.class)) {
+                errMsg = "There was error in trying to parse value: " + value + ". Please enter a valid double value for parameter " + name;
+                Double.parseDouble(value);
+            }
+        } catch (final Exception e) {
+            // catching generic exception as some throws NullPointerException and some throws NumberFormatExcpeion
+            s_logger.error(errMsg);
+            return errMsg;
+        }
+        return errMsg;
+    }
+
     /**
      * A valid value should be an integer between min and max (the values from the range).
      */

server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java

@@ -155,7 +155,7 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService,
     public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
         _name = name;
         String elbEnabledString = _configDao.getValue(Config.ElasticLoadBalancerEnabled.key());
-        _elbEnabled = Boolean.parseBoolean(elbEnabledString);
+        _elbEnabled = elbEnabledString == null ? false : Boolean.parseBoolean(elbEnabledString);
         if (_ipAddrMgr.RulesContinueOnError.value() != null) {
             rulesContinueOnErrFlag = _ipAddrMgr.RulesContinueOnError.value();
         }

server/src/main/java/com/cloud/storage/ImageStoreServiceImpl.java

@@ -32,7 +32,6 @@
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
-import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.jobs.AsyncJobManager;
 import org.apache.cloudstack.storage.ImageStoreService;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
@@ -59,18 +58,6 @@ public class ImageStoreServiceImpl extends ManagerBase implements ImageStoreServ
     @Inject
     public UUIDManager uuidMgr;
 
-    ConfigKey<Double> ImageStoreImbalanceThreshold = new ConfigKey<>("Advanced", Double.class,
-            "image.store.imbalance.threshold",
-            "0.3",
-            "The storage imbalance threshold that is compared with the standard deviation percentage for a storage utilization metric. " +
-                    "The value is a percentage in decimal format.",
-            true, ConfigKey.Scope.Global);
-
-
-    public Integer numConcurrentCopyTasksPerSSVM = null;
-
-
-
     @Override
     public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
         return true;