diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java b/core/src/main/java/org/apache/accumulo/core/conf/Property.java index b148c1a60e5..7ad9face671 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java +++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java @@ -1564,6 +1564,23 @@ public Property replacedBy() { return replacedBy; } + /** + * Gets the defined required properties + * + * @return Set{@literal } + */ + public static Set getRequiredProperties() { + return Set.of(Property.INSTANCE_ZK_HOST, Property.INSTANCE_ZK_TIMEOUT, Property.INSTANCE_SECRET, + Property.INSTANCE_VOLUMES, Property.GENERAL_THREADPOOL_SIZE, + Property.GENERAL_DELEGATION_TOKEN_LIFETIME, + Property.GENERAL_DELEGATION_TOKEN_UPDATE_INTERVAL, Property.GENERAL_IDLE_PROCESS_INTERVAL, + Property.GENERAL_LOW_MEM_DETECTOR_INTERVAL, Property.GENERAL_LOW_MEM_DETECTOR_THRESHOLD, + Property.GENERAL_SERVER_LOCK_VERIFICATION_INTERVAL, Property.MANAGER_CLIENTPORT, + Property.TSERV_CLIENTPORT, Property.GC_CYCLE_START, Property.GC_CYCLE_DELAY, + Property.GC_PORT, Property.MONITOR_PORT, Property.TABLE_MAJC_RATIO, + Property.TABLE_SPLIT_THRESHOLD); + } + private void precomputeAnnotations() { isSensitive = hasAnnotation(Sensitive.class) || hasPrefixWithAnnotation(getKey(), Sensitive.class); diff --git a/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java b/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java index 2f92a3d4c83..aafe0edf207 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java +++ b/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java @@ -21,6 +21,8 @@ import static java.util.Objects.requireNonNull; import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; import java.util.Arrays; import java.util.HashSet; import java.util.Objects; @@ -114,7 +116,7 @@ public enum PropertyType { + " '5%', '0.2%', '0.0005'.\n" + "Examples of invalid fractions/percentages are '', '10 percent', 'Hulk Hogan'"), - PATH("path", x -> true, + PATH("path", new ValidPath(), "A string that represents a filesystem path, which can be either relative" + " or absolute to some directory. The filesystem depends on the property. " + "Substitutions of the ACCUMULO_HOME environment variable can be done in the system " @@ -155,7 +157,7 @@ public enum PropertyType { BOOLEAN("boolean", in(false, null, "true", "false"), "Has a value of either 'true' or 'false' (case-insensitive)"), - URI("uri", x -> true, "A valid URI"), + URI("uri", new ValidUri(), "A valid URI"), FILENAME_EXT("file name extension", in(true, RFile.EXTENSION), "One of the currently supported filename extensions for storing table data files. " @@ -211,6 +213,46 @@ public boolean isValidFormat(String value) { return predicate.test(value); } + /** + * Validate that the provided string is a valid path. + */ + private static class ValidPath implements Predicate { + private static final Logger log = LoggerFactory.getLogger(ValidPath.class); + + @Override + public boolean test(String path) { + if (path == null || path.trim().isEmpty()) { + return true; + } else if (new Path(path.trim()).isAbsolute()) { + return true; + } else if (path.matches("/?[A-Za-z+/?]+")) { + return true; + } + log.error("provided path is not valid"); + return false; + } + } + + // SECOND VERSION OF ValidPath, leaving while waiting for clarification on the expected validation + /** + * Validate that the provided string is a valid hadoop path. Path must exist and be a valid + * file/directory + */ + /* + * private static class ValidPath implements Predicate { private static final Logger log = + * LoggerFactory.getLogger(ValidPath.class); + * + * @Override public boolean test(String path) { Configuration conf = new Configuration(); Path + * hadoopPath = new Path(path); + * + * try { FileSystem fs = hadoopPath.getFileSystem(conf); // Check if path exists if + * (fs.exists(hadoopPath)) { // Check if path is a valid directory if + * (fs.getFileStatus(hadoopPath).isFile() || fs.getFileStatus(hadoopPath).isDirectory()) { return + * true; } log.error("provided path is not a file or directory"); return false; } + * log.error("provided path does not exist"); return false; } catch (IOException e) { + * log.error("provided path is not valid"); return false; } } } + */ + /** * Validate that the provided string can be parsed into a json object. This implementation uses * jackson databind because it is less permissive that GSON for what is considered valid. This @@ -247,6 +289,24 @@ public boolean test(String value) { } } + private static class ValidUri implements Predicate { + private static final Logger log = LoggerFactory.getLogger(ValidUri.class); + + @Override + public boolean test(String uri) { + if (uri == null) { + return true; + } + try { + new URI(uri); + return true; + } catch (URISyntaxException e) { + log.error("provided uri string is not valid"); + return false; + } + } + } + private static class ValidVolumes implements Predicate { private static final Logger log = LoggerFactory.getLogger(ValidVolumes.class); @@ -306,7 +366,6 @@ public boolean test(String type) { } } } - } private static final Pattern SUFFIX_REGEX = Pattern.compile("\\D*$"); // match non-digits at end @@ -413,14 +472,8 @@ public Matches(final Pattern pattern) { @Override public boolean test(final String input) { - // TODO when the input is null, it just means that the property wasn't set - // we can add checks for not null for required properties with - // Predicates.and(Predicates.notNull(), ...), - // or we can stop assuming that null is always okay for a Matches predicate, and do that - // explicitly with Predicates.or(Predicates.isNull(), ...) return input == null || pattern.matcher(input).matches(); } - } public static class PortRange extends Matches { diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/ServerConfigCheckRunner.java b/server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/ServerConfigCheckRunner.java index a13de0e59dc..932895427cb 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/ServerConfigCheckRunner.java +++ b/server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/ServerConfigCheckRunner.java @@ -52,19 +52,7 @@ public boolean runCheck(ServerContext context, ServerOpts opts, boolean fixFiles } log.trace("Checking that all required config properties are present"); - // there are many properties that should be set (default value or user set), identifying them - // all and checking them here is unrealistic. Some property that is not set but is expected - // will likely result in some sort of failure eventually anyway. We will just check a few - // obvious required properties here. - Set requiredProps = Set.of(Property.INSTANCE_ZK_HOST, Property.INSTANCE_ZK_TIMEOUT, - Property.INSTANCE_SECRET, Property.INSTANCE_VOLUMES, Property.GENERAL_THREADPOOL_SIZE, - Property.GENERAL_DELEGATION_TOKEN_LIFETIME, - Property.GENERAL_DELEGATION_TOKEN_UPDATE_INTERVAL, Property.GENERAL_IDLE_PROCESS_INTERVAL, - Property.GENERAL_LOW_MEM_DETECTOR_INTERVAL, Property.GENERAL_LOW_MEM_DETECTOR_THRESHOLD, - Property.GENERAL_SERVER_LOCK_VERIFICATION_INTERVAL, Property.MANAGER_CLIENTPORT, - Property.TSERV_CLIENTPORT, Property.GC_CYCLE_START, Property.GC_CYCLE_DELAY, - Property.GC_PORT, Property.MONITOR_PORT, Property.TABLE_MAJC_RATIO, - Property.TABLE_SPLIT_THRESHOLD); + Set requiredProps = Property.getRequiredProperties(); for (var reqProp : requiredProps) { var confPropVal = config.get(reqProp); // already checked that all set properties are valid, just check that it is set then we know