diff --git a/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationDocGen.java b/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationDocGen.java
index 4595123deb7..f2e4705cf0b 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationDocGen.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationDocGen.java
@@ -115,17 +115,19 @@ void property(Property prop) {
description += strike(sanitize(prop.getDescription()), depr) + "
"
+ strike("**type:** " + prop.getType().name(), depr) + ", "
+ strike("**zk mutable:** " + isZooKeeperMutable(prop), depr) + ", ";
- String defaultValue = sanitize(prop.getDefaultValue()).trim();
- if (defaultValue.isEmpty()) {
- description += strike("**default value:** empty", depr);
- } else if (defaultValue.contains("\n")) {
- // deal with multi-line values, skip strikethrough of value
- description += strike("**default value:** ", depr) + "\n```\n" + defaultValue + "\n```\n";
- } else if (prop.getType() == PropertyType.CLASSNAME
- && defaultValue.startsWith("org.apache.accumulo")) {
- description += strike("**default value:** {% jlink -f " + defaultValue + " %}", depr);
- } else {
- description += strike("**default value:** `" + defaultValue + "`", depr);
+ if (prop.getDefaultValue() != null) {
+ String defaultValue = sanitize(prop.getDefaultValue()).trim();
+ if (defaultValue.isEmpty()) {
+ description += strike("**default value:** empty", depr);
+ } else if (defaultValue.contains("\n")) {
+ // deal with multi-line values, skip strikethrough of value
+ description += strike("**default value:** ", depr) + "\n```\n" + defaultValue + "\n```\n";
+ } else if (prop.getType() == PropertyType.CLASSNAME
+ && defaultValue.startsWith("org.apache.accumulo")) {
+ description += strike("**default value:** {% jlink -f " + defaultValue + " %}", depr);
+ } else {
+ description += strike("**default value:** `" + defaultValue + "`", depr);
+ }
}
doc.println("| " + key + " | " + description + " |");
}
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..c73d02c83c7 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
@@ -161,7 +161,7 @@ public enum Property {
HDFS. To use the ChangeSecret tool, run the command: `./bin/accumulo \
admin changeSecret`.
""", "1.3.5"),
- INSTANCE_VOLUMES("instance.volumes", "", PropertyType.VOLUMES, """
+ INSTANCE_VOLUMES("instance.volumes", null, PropertyType.VOLUMES, """
A comma separated list of dfs uris to use. Files will be stored across \
these filesystems. In some situations, the first volume in this list \
may be treated differently, such as being preferred for writing out \
@@ -1437,14 +1437,22 @@ start with the category prefix, followed by a scope (minc, majc, scan, \
private boolean isReplaced;
private Property replacedBy = null;
private final PropertyType type;
+ private final boolean isRequired;
+ // private final Predicate checkValue;
Property(String name, String defaultValue, PropertyType type, String description,
String availableSince) {
this.key = name;
this.defaultValue = defaultValue;
+ this.type = type;
this.description = description;
this.availableSince = availableSince;
- this.type = type;
+ isRequired = this.defaultValue == null;
+ /*
+ * checkValue = isRequired ? ((Predicate)
+ * Objects::nonNull).and(Predicate.not(String::isEmpty)) .and(type::isValidFormat) :
+ * ((Predicate) Objects::isNull).or(String::isEmpty).or(type::isValidFormat);
+ */
}
@Override
@@ -1480,6 +1488,15 @@ public PropertyType getType() {
return this.type;
}
+ /**
+ * Gets isRequired of this property.
+ *
+ * @return isRequired
+ */
+ public boolean getIsRequired() {
+ return this.isRequired;
+ }
+
/**
* Gets the description of this property.
*
@@ -1641,7 +1658,7 @@ public static boolean isValidProperty(final String key, final String value) {
// If a key doesn't exist yet, then check if it follows a valid prefix
return validPrefixes.stream().anyMatch(key::startsWith);
}
- return (isValidPropertyKey(key) && p.getType().isValidFormat(value));
+ return isValidPropertyKey(key) && p.getType().isValidFormat(value);
}
/**
@@ -1653,7 +1670,6 @@ public static boolean isValidProperty(final String key, final String value) {
*/
public static boolean isValidPropertyKey(String key) {
return validProperties.contains(key) || validPrefixes.stream().anyMatch(key::startsWith);
-
}
/**
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..02464bc2107 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;
@@ -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. "
@@ -247,12 +249,35 @@ public boolean test(String value) {
}
}
+ /**
+ * Validate that the provided string can be used to create a valid URI.
+ */
+ 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);
@Override
public boolean test(String volumes) {
if (volumes == null) {
+ return true;
+ } else if (volumes.isEmpty()) {
return false;
}
try {
@@ -306,7 +331,6 @@ public boolean test(String type) {
}
}
}
-
}
private static final Pattern SUFFIX_REGEX = Pattern.compile("\\D*$"); // match non-digits at end
@@ -413,14 +437,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/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java b/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
index be100685e85..b82345aa681 100644
--- a/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
@@ -52,6 +52,7 @@ public void testProperties() {
for (Property prop : Property.values()) {
if (prop.getType().equals(PropertyType.PREFIX)) {
validPrefixes.add(prop.getKey());
+ // assertFalse(prop.getIsRequired());
}
}
@@ -63,18 +64,19 @@ public void testProperties() {
"PREFIX property " + prop.name() + " has unexpected non-null default value.");
} else {
// default values shouldn't be null, but they can be an empty string
- assertNotNull(prop.getDefaultValue());
- // default values shouldn't start or end with whitespace
- assertEquals(prop.getDefaultValue().strip(), prop.getDefaultValue(),
- "Property " + prop.name() + " starts or ends with whitespace");
- // default values shouldn't contain newline characters or tabs
- assertFalse(prop.getDefaultValue().contains("\t"),
- "Property " + prop.name() + " contains a tab character");
- assertFalse(prop.getDefaultValue().contains("\n"),
- "Property " + prop.name() + " contains a newline (\\n) character");
- assertFalse(prop.getDefaultValue().contains("\r"),
- "Property " + prop.name() + " contains a return (\\r) character");
-
+ if (!prop.getIsRequired()) {
+ assertNotNull(prop.getDefaultValue());
+ // default values shouldn't start or end with whitespace
+ assertEquals(prop.getDefaultValue().strip(), prop.getDefaultValue(),
+ "Property " + prop.name() + " starts or ends with whitespace");
+ // default values shouldn't contain newline characters or tabs
+ assertFalse(prop.getDefaultValue().contains("\t"),
+ "Property " + prop.name() + " contains a tab character");
+ assertFalse(prop.getDefaultValue().contains("\n"),
+ "Property " + prop.name() + " contains a newline (\\n) character");
+ assertFalse(prop.getDefaultValue().contains("\r"),
+ "Property " + prop.name() + " contains a return (\\r) character");
+ }
assertTrue(Property.isValidProperty(prop.getKey(), prop.getDefaultValue()),
"Property " + prop.name() + " has invalid default value " + prop.getDefaultValue()
+ " for type " + prop.getType());
diff --git a/core/src/test/java/org/apache/accumulo/core/conf/PropertyTypeTest.java b/core/src/test/java/org/apache/accumulo/core/conf/PropertyTypeTest.java
index 86a1037f85f..c499ff1b577 100644
--- a/core/src/test/java/org/apache/accumulo/core/conf/PropertyTypeTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/conf/PropertyTypeTest.java
@@ -207,6 +207,9 @@ public void testTypePREFIX() {
public void testTypeSTRING() {
valid(null, "", "whatever");
}
+ /*
+ * @Test public void testTypeNON_EMPTY_STRING() { valid("whatever"); invalid(null, "", " "); }
+ */
@Test
public void testTypeTIMEDURATION() {
@@ -228,9 +231,9 @@ public void testTypeFILENAME_EXT() {
@Test
public void testTypeVOLUMES() {
// more comprehensive parsing tests are in ConfigurationTypeHelperTest.testGetVolumeUris()
- valid("", "hdfs:/volA", ",hdfs:/volA", "hdfs:/volA,", "hdfs:/volA,file:/volB",
+ valid(null, "hdfs:/volA", ",hdfs:/volA", "hdfs:/volA,", "hdfs:/volA,file:/volB",
",hdfs:/volA,file:/volB", "hdfs:/volA,,file:/volB", "hdfs:/volA,file:/volB, ,");
- invalid(null, " ", ",", ",,,", " ,,,", ",,, ", ", ,,", "hdfs:/volA,hdfs:/volB,volA",
+ invalid("", " ", ",", ",,,", " ,,,", ",,, ", ", ,,", "hdfs:/volA,hdfs:/volB,volA",
",volA,hdfs:/volA,hdfs:/volB", "hdfs:/volA,,volA,hdfs:/volB",
"hdfs:/volA,volA,hdfs:/volB, ,", "hdfs:/volA,hdfs:/volB,hdfs:/volA",
"hdfs:/volA,hdfs :/::/volB");
diff --git a/core/src/test/java/org/apache/accumulo/core/conf/SiteConfigurationTest.java b/core/src/test/java/org/apache/accumulo/core/conf/SiteConfigurationTest.java
index e97c8a1615a..f9c2723fb35 100644
--- a/core/src/test/java/org/apache/accumulo/core/conf/SiteConfigurationTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/conf/SiteConfigurationTest.java
@@ -59,7 +59,7 @@ public void testDefault() {
var conf = SiteConfiguration.empty().build();
assertEquals("localhost:2181", conf.get(Property.INSTANCE_ZK_HOST));
assertEquals("DEFAULT", conf.get(Property.INSTANCE_SECRET));
- assertEquals("", conf.get(Property.INSTANCE_VOLUMES));
+ assertNull(conf.get(Property.INSTANCE_VOLUMES.getDefaultValue()));
assertEquals("120s", conf.get(Property.GENERAL_RPC_TIMEOUT));
assertEquals("1G", conf.get(Property.TSERV_WAL_MAX_SIZE));
assertEquals("org.apache.accumulo.core.spi.crypto.NoCryptoServiceFactory",
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..ae34cad3757 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
@@ -20,7 +20,6 @@
import java.util.HashMap;
import java.util.Map;
-import java.util.Set;
import org.apache.accumulo.core.cli.ServerOpts;
import org.apache.accumulo.core.conf.Property;
@@ -47,34 +46,9 @@ public boolean runCheck(ServerContext context, ServerOpts opts, boolean fixFiles
var val = entry.getValue();
if (!Property.isValidProperty(key, val)) {
log.warn("Invalid property (key={} val={}) found in the config", key, val);
- status &= false;
+ status = false;
}
}
-
- 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);
- 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
- // it's valid
- if (confPropVal == null || confPropVal.isEmpty()) {
- log.warn("Required property {} is not set!", reqProp);
- status &= false;
- }
- }
-
printCompleted(status);
return status;
}