You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/12/22 14:22:53 UTC
[GitHub] [cloudstack] stephankruggg commented on a diff in pull request #6808: Allow privateips on console proxy
stephankruggg commented on code in PR #6808:
URL: https://github.com/apache/cloudstack/pull/6808#discussion_r1055497800
##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -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.
Review Comment:
```suggestion
* Checks if the value for the configuration is valid for any of the ranges selected.
```
##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -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 (StringUtils.isEmpty(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)";
Review Comment:
```suggestion
return "a valid instance name (instance names cannot contain hyphens, spaces or plus signs)";
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org