From a702db0ec146134aaf6435b41941fef08f9b652b Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Tue, 21 Jul 2026 12:49:52 -0500 Subject: [PATCH 1/7] Move required properties out of ServerConfigCheckRunner --- .../org/apache/accumulo/core/conf/Property.java | 17 +++++++++++++++++ .../checkCommand/ServerConfigCheckRunner.java | 14 +------------- 2 files changed, 18 insertions(+), 13 deletions(-) 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..132679665b2 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 requiredProperties + */ + 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/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 From a44536bb8596fb83730fc16b08342e2596dc9cfa Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Tue, 21 Jul 2026 12:54:29 -0500 Subject: [PATCH 2/7] Fix incorrect return type in comment --- core/src/main/java/org/apache/accumulo/core/conf/Property.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 132679665b2..ad505d515d5 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 @@ -1567,7 +1567,7 @@ public Property replacedBy() { /** * Gets the defined required properties * - * @return requiredProperties + * @return Set */ public static Set getRequiredProperties() { return Set.of(Property.INSTANCE_ZK_HOST, Property.INSTANCE_ZK_TIMEOUT, From f9d82befcb1c91b192e5e377c55ec96f119430c2 Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Wed, 22 Jul 2026 13:28:35 -0500 Subject: [PATCH 3/7] Add validation for PropertyType.URI and PropertyType.PATH --- .../apache/accumulo/core/conf/Property.java | 18 +++--- .../accumulo/core/conf/PropertyType.java | 59 ++++++++++++++++++- 2 files changed, 65 insertions(+), 12 deletions(-) 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 ad505d515d5..4749b3de081 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 @@ -1570,15 +1570,15 @@ public Property replacedBy() { * @return Set */ 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); + 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() { 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..2711b964646 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; @@ -38,6 +40,8 @@ import org.apache.accumulo.core.file.rfile.bcfile.Compression; import org.apache.accumulo.core.file.rfile.bcfile.CompressionAlgorithm; import org.apache.commons.lang3.Range; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.io.compress.Compressor; import org.slf4j.Logger; @@ -114,7 +118,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 +159,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 +215,38 @@ public boolean isValidFormat(String value) { return predicate.test(value); } + /** + * 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 +283,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 false; + } + 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 +360,6 @@ public boolean test(String type) { } } } - } private static final Pattern SUFFIX_REGEX = Pattern.compile("\\D*$"); // match non-digits at end From a77bedb7ae40255df5370240b3445801a185f28b Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Wed, 22 Jul 2026 15:10:51 -0500 Subject: [PATCH 4/7] Fix failing unit test for PropertyType.PATH --- .../accumulo/core/conf/PropertyType.java | 62 ++++++++++++------- 1 file changed, 38 insertions(+), 24 deletions(-) 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 2711b964646..0ec545238f3 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 @@ -40,8 +40,6 @@ import org.apache.accumulo.core.file.rfile.bcfile.Compression; import org.apache.accumulo.core.file.rfile.bcfile.CompressionAlgorithm; import org.apache.commons.lang3.Range; -import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.io.compress.Compressor; import org.slf4j.Logger; @@ -216,37 +214,53 @@ public boolean isValidFormat(String value) { } /** - * Validate that the provided string is a valid hadoop path. Path must exist and be a valid - * file/directory + * 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) { - 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; + if (path == null || path.trim().isEmpty()) { + return true; } + // path is absolute + else if (new Path(path.trim()).isAbsolute()) { + return true; + } + // path with one .../... + else if (path.matches("[A-Za-z]+/?[A-Za-z]+")) { + return true; + } + // path with /.../.../ + 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 @@ -289,7 +303,7 @@ private static class ValidUri implements Predicate { @Override public boolean test(String uri) { if (uri == null) { - return false; + return true; } try { new URI(uri); From b993313989c3aeffd27de652b94be6ec641a9a7f Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Thu, 23 Jul 2026 14:47:21 -0500 Subject: [PATCH 5/7] Fix checkstyle violdations --- .../org/apache/accumulo/core/conf/Property.java | 2 +- .../apache/accumulo/core/conf/PropertyType.java | 16 ++++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) 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 4749b3de081..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 @@ -1567,7 +1567,7 @@ public Property replacedBy() { /** * Gets the defined required properties * - * @return Set + * @return Set{@literal } */ public static Set getRequiredProperties() { return Set.of(Property.INSTANCE_ZK_HOST, Property.INSTANCE_ZK_TIMEOUT, Property.INSTANCE_SECRET, 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 0ec545238f3..c86edbff2d1 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 @@ -48,6 +48,7 @@ import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Preconditions; +import com.google.common.base.Predicates; import com.google.gson.JsonParser; /** @@ -223,17 +224,11 @@ private static class ValidPath implements Predicate { public boolean test(String path) { if (path == null || path.trim().isEmpty()) { return true; - } - // path is absolute - else if (new Path(path.trim()).isAbsolute()) { + } else if (new Path(path.trim()).isAbsolute()) { return true; - } - // path with one .../... - else if (path.matches("[A-Za-z]+/?[A-Za-z]+")) { + } else if (path.matches("[A-Za-z]+/?[A-Za-z]+")) { return true; - } - // path with /.../.../ - else if (path.matches("/?[A-Za-z+/?]+")) { + } else if (path.matches("/?[A-Za-z+/?]+")) { return true; } log.error("provided path is not valid"); @@ -485,9 +480,10 @@ public boolean test(final String input) { // 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(), ...) + + final Predicate notNullPredicate = Predicates.notNull(); return input == null || pattern.matcher(input).matches(); } - } public static class PortRange extends Matches { From c499e66808cc77743135da3b37b822719c7eaade Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Thu, 23 Jul 2026 15:57:58 -0500 Subject: [PATCH 6/7] Remove outdated TODO from PropertyType.java, apache#2699 --- .../java/org/apache/accumulo/core/conf/PropertyType.java | 8 -------- 1 file changed, 8 deletions(-) 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 c86edbff2d1..2a74cc17aba 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 @@ -48,7 +48,6 @@ import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Preconditions; -import com.google.common.base.Predicates; import com.google.gson.JsonParser; /** @@ -475,13 +474,6 @@ 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(), ...) - - final Predicate notNullPredicate = Predicates.notNull(); return input == null || pattern.matcher(input).matches(); } } From 335922d884c4a99e08ad05ff9879e410759eb76d Mon Sep 17 00:00:00 2001 From: Amanda Villarreal Date: Thu, 23 Jul 2026 16:35:42 -0500 Subject: [PATCH 7/7] Simplify matches regex for PropertyType.PATH validation --- .../main/java/org/apache/accumulo/core/conf/PropertyType.java | 2 -- 1 file changed, 2 deletions(-) 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 2a74cc17aba..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 @@ -225,8 +225,6 @@ public boolean test(String path) { return true; } else if (new Path(path.trim()).isAbsolute()) { return true; - } else if (path.matches("[A-Za-z]+/?[A-Za-z]+")) { - return true; } else if (path.matches("/?[A-Za-z+/?]+")) { return true; }