create parameter to determine whether roles are public or private

Author: Gabriel Ortiga Fernandes

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

@@ -244,7 +244,7 @@ public enum Config {
             String.class,
             "network.loadbalancer.haproxy.stats.auth",
             "admin1:AdMiN123",
-            "Load Balancer(haproxy) authetication string in the format username:password",
+            "Load Balancer(haproxy) authentication string in the format username:password",
             null),
     NetworkLBHaproxyStatsPort(
             "Network",
@@ -426,7 +426,7 @@ public enum Config {
             "Console proxy command port that is used to communicate with management server",
             null),
     ConsoleProxyRestart("Console Proxy", AgentManager.class, Boolean.class, "consoleproxy.restart", "true", "Console proxy restart flag, defaulted to true", null),
-    ConsoleProxyUrlDomain("Console Proxy", AgentManager.class, String.class, "consoleproxy.url.domain", "", "Console proxy url domain", "domainName"),
+    ConsoleProxyUrlDomain("Console Proxy", AgentManager.class, String.class, "consoleproxy.url.domain", "", "Console proxy url domain", "domainName","privateip"),
     ConsoleProxySessionMax(
             "Console Proxy",
             AgentManager.class,
@@ -1783,7 +1783,7 @@ public enum Config {
     private final String _name;
     private final String _defaultValue;
     private final String _description;
-    private final String _range;
+    private final String[] _range;
     private final String _scope; // Parameter can be at different levels (Zone/cluster/pool/account), by default every parameter is at global
 
     private static final HashMap<String, List<Config>> s_scopeLevelConfigsMap = new HashMap<String, List<Config>>();
@@ -1833,7 +1833,7 @@ public enum Config {
         }
     }
 
-    private Config(String category, Class<?> componentClass, Class<?> type, String name, String defaultValue, String description, String range) {
+    private Config(String category, Class<?> componentClass, Class<?> type, String name, String defaultValue, String description, String... range) {
         _category = category;
         _componentClass = componentClass;
         _type = type;
@@ -1896,7 +1896,7 @@ public String getComponent() {
         }
     }
 
-    public String getRange() {
+    public String[] getRange() {
         return _range;
     }
 

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

@@ -1103,8 +1103,8 @@ private String validateConfigurationValue(final String name, String value, final
             }
         }
         Class<?> type = null;
-        final Config c = Config.getConfig(name);
-        if (c == null) {
+        final Config configuration = Config.getConfig(name);
+        if (configuration == null) {
             s_logger.warn("Did not find configuration " + name + " in Config.java. Perhaps moved to ConfigDepot");
             final ConfigKey<?> configKey = _configDepot.get(name);
             if(configKey == null) {
@@ -1113,7 +1113,7 @@ private String validateConfigurationValue(final String name, String value, final
             }
             type = configKey.type();
         } else {
-            type = c.getType();
+            type = configuration.getType();
         }
         //no need to validate further if a
         //config can have null value.
@@ -1222,96 +1222,145 @@ private String validateConfigurationValue(final String name, String value, final
             }
         }
 
-        if(c == null ) {
+        if (configuration == null ) {
             //range validation has to be done per case basis, for now
             //return in case of Configkey parameters
             return null;
         }
 
-        final String range = c.getRange();
+        final String[] range = configuration.getRange();
         if (range == null) {
             return null;
         }
 
         if (type.equals(String.class)) {
-            if (range.equals("privateip")) {
-                try {
-                    if (!NetUtils.isSiteLocalAddress(value)) {
-                        s_logger.error("privateip range " + value + " is not a site local address for configuration variable " + name);
-                        return "Please enter a site local IP address.";
-                    }
-                } catch (final NullPointerException e) {
-                    s_logger.error("Error parsing ip address for " + name);
-                    throw new InvalidParameterValueException("Error parsing ip address");
-                }
-            } else if (range.equals("netmask")) {
-                if (!NetUtils.isValidIp4Netmask(value)) {
-                    s_logger.error("netmask " + value + " is not a valid net mask for configuration variable " + name);
-                    return "Please enter a valid netmask.";
-                }
-            } else if (range.equals("hypervisorList")) {
-                final String[] hypervisors = value.split(",");
-                if (hypervisors == null) {
-                    return "Please enter hypervisor list, separated by comma";
-                }
-                for (final String hypervisor : hypervisors) {
-                    if (HypervisorType.getType(hypervisor) == HypervisorType.Any || HypervisorType.getType(hypervisor) == HypervisorType.None) {
-                        return "Please enter a valid hypervisor type";
-                    }
-                }
-            } else if (range.equalsIgnoreCase("instanceName")) {
-                if (!NetUtils.verifyInstanceName(value)) {
-                    return "Instance name can not contain hyphen, space or plus sign";
-                }
-            } else if (range.equalsIgnoreCase("domainName")) {
-                String domainName = value;
-                if (value.startsWith("*")) {
-                    domainName = value.substring(2); //skip the "*."
-                }
-                //max length for FQDN is 253 + 2, code adds xxx-xxx-xxx-xxx to domain name when creating URL
-                if (domainName.length() >= 238 || !domainName.matches(DOMAIN_NAME_PATTERN)) {
-                    return "Please enter a valid string for domain name, prefixed with '*.' if applicable";
-                }
-            } else if (range.equals("routes")) {
-                final String[] routes = value.split(",");
-                for (final String route : routes) {
-                    if (route != null) {
-                        final String routeToVerify = route.trim();
-                        if (!NetUtils.isValidIp4Cidr(routeToVerify)) {
-                            throw new InvalidParameterValueException("Invalid value for route: " + route + " in deny list. Valid format is list"
-                                    + " of cidrs separated by coma. Example: 10.1.1.0/24,192.168.0.0/24");
-                        }
-                    }
-                }
-            } else {
-                final String[] options = range.split(",");
-                for (final String option : options) {
-                    if (option.trim().equalsIgnoreCase(value)) {
-                        return null;
-                    }
-                }
-                s_logger.error("configuration value for " + name + " is invalid");
-                return "Please enter : " + range;
+            return validateIfStringValueIsInRange(name, value, range);
+        } else if (type.equals(Integer.class)) {
+            return validateIfIntValueIsInRange(name, value, range[0]);
+        }
+        return String.format("Invalid value for configuration [%s].", name);
+    }
 
+    /**
+     * A valid value should be an integer between min and max (the values from the range).
+     */
+    protected String validateIfIntValueIsInRange(String name, String value, String range) {
+        final String[] options = range.split("-");
+        final int min = Integer.parseInt(options[0]);
+        final int max = Integer.parseInt(options[1]);
+        final int val = Integer.parseInt(value);
+        if (val < min || val > max) {
+            s_logger.error(String.format("Invalid value for configuration [%s]. Please enter a value in the range [%s].", name, range));
+            return String.format("The provided value is not valid for this configuration. Please enter an integer in the range: [%s]", range);
+        }
+        return null;
+    }
+
+    /**
+     * This method checks if the value for the configuration is valid for any of the ranges selected.
+     */
+    protected String validateIfStringValueIsInRange(String name, String value, String... range) {
+        List<String> message = new ArrayList<String>();
+        String errMessage = "";
+        for (String rangeOption : range) {
+            switch (rangeOption) {
+                case "privateip":
+                    errMessage = validateRangePrivateIp(name, value);
+                    break;
+                case "hypervisorList":
+                    errMessage = validateRangeHypervisorList(value);
+                    break;
+                case "instanceName":
+                    errMessage = validateRangeInstanceName(value);
+                    break;
+                case "domainName":
+                    errMessage = validateRangeDomainName(value);
+                    break;
+                default:
+                    errMessage = validateRangeOther(name, value, rangeOption);
+            }
+            if (Strings.isNullOrEmpty(errMessage)) {
+                return null;
             }
-        } else if (type.equals(Integer.class)) {
-            final String[] options = range.split("-");
-            if (options.length != 2) {
-                final String msg = "configuration range " + range + " for " + name + " is invalid";
-                s_logger.error(msg);
-                return msg;
+            message.add(errMessage);
+        }
+        if (message.size() == 1) {
+            return String.format("The provided value is not %s.", message.get(0));
+        }
+        return String.format("The provided value is neither %s.", String.join(" NOR ", message));
+    }
+
+    /**
+     * Checks if the value is a private IP according to {@link NetUtils#isSiteLocalAddress(String)}.
+     */
+    protected String validateRangePrivateIp(String name, String value) {
+        try {
+            if (NetUtils.isSiteLocalAddress(value)) {
+                return null;
             }
-            final int min = Integer.parseInt(options[0]);
-            final int max = Integer.parseInt(options[1]);
-            final int val = Integer.parseInt(value);
-            if (val < min || val > max) {
-                s_logger.error("configuration value for " + name + " is invalid");
-                return "Please enter : " + range;
+            s_logger.error(String.format("Value [%s] is not a valid private IP range for configuration [%s].", value, name));
+        } catch (final NullPointerException e) {
+            s_logger.error(String.format("Error while parsing IP address for [%s].", name));
+        }
+        return "a valid site local IP address";
+    }
+
+    /**
+     * Valid values are XenServer, KVM, VMware, Hyperv, VirtualBox, Parralels, BareMetal, Simulator, Ovm, Ovm3, LXC.
+     * Inputting "Any" will return the hypervisor type Any, other inputs will result in the hypervisor type none.
+     * Both of these are invalid values and will return an error message.
+     */
+    protected String validateRangeHypervisorList(String value) {
+        final String[] hypervisors = value.split(",");
+        for (final String hypervisor : hypervisors) {
+            if (HypervisorType.getType(hypervisor) == HypervisorType.Any || HypervisorType.getType(hypervisor) == HypervisorType.None) {
+                return "a valid hypervisor type";
             }
         }
         return null;
     }
 
+    /**
+     * Valid values are instance names, the only restriction is that they may not have hyphens, spaces or plus signs.
+     */
+    protected String validateRangeInstanceName(String value) {
+        if (NetUtils.verifyInstanceName(value)) {
+            return null;
+        }
+        return "a valid instance name (instance names cannot contain hyphen, space or plus sign)";
+    }
+
+    /**
+     * Verifies if the value is a valid domain name. If it starts with "*.", these two symbols are ignored and do not count towards the character limit.
+     * Max length for FQDN is 253 + 2, code adds xxx-xxx-xxx-xxx to domain name when creating URL.
+     */
+    protected String validateRangeDomainName(String value) {
+        String domainName = value;
+        if (value.startsWith("*")) {
+            domainName = value.substring(2);
+        }
+        if (domainName.length() >= 238 || !domainName.matches(DOMAIN_NAME_PATTERN)) {
+            return "a valid domain name";
+        }
+        return null;
+    }
+
+    /**
+     * In configurations where this type of range is used, a list of possible values is passed as argument in the creation of the configuration,
+     * a valid value is any option within this list.
+     */
+    protected String validateRangeOther(String name, String value, String rangeOption) {
+        final String[] options = rangeOption.split(",");
+        for (final String option : options) {
+            if (option.trim().equalsIgnoreCase(value)) {
+                return null;
+            }
+        }
+        s_logger.error(String.format("Invalid value for configuration [%s].", name));
+        return String.format("a valid value for this configuration (Options are: [%s])", rangeOption);
+    }
+
+
     private boolean podHasAllocatedPrivateIPs(final long podId) {
         final HostPodVO pod = _podDao.findById(podId);
         final int count = _privateIpAddressDao.countIPs(podId, pod.getDataCenterId(), true);

server/src/test/java/com/cloud/configuration/ConfigTest.java

@@ -0,0 +1,42 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.configuration;
+
+import org.junit.Test;
+
+public class ConfigTest {
+    @Test
+    public void configTestIntegerRange() {
+        for (Config configuration : Config.values()) {
+            if (configuration.getType().equals(Integer.class) && configuration.getRange() != null) {
+                try {
+                    final String[] options = configuration.getRange()[0].split("-");
+                    final int min = Integer.parseInt(options[0]);
+                    final int max = Integer.parseInt(options[1]);
+                    if (options.length != 2) {
+                        throw new AssertionError(String.format("Invalid range for configuration [%s], a valid value for the range should be two integers separated by [-].", configuration.toString()));
+                    }
+                    if (min > max) {
+                        throw new AssertionError(String.format("Invalid range for configuration [%s], the second value should be greater than the first.", configuration.toString()));
+                    }
+                } catch (java.lang.NumberFormatException e) {
+                    throw new AssertionError(String.format("Invalid range for configuration [%s], a valid value for the range should be two integers separated by [-].", configuration.toString()));
+                }
+            }
+        }
+    }
+}