diff --git a/core/pom.xml b/core/pom.xml index b04b47f5658..68de14175be 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -137,6 +137,10 @@ org.apache.zookeeper zookeeper-jute + + org.checkerframework + checker-qual + org.slf4j slf4j-api diff --git a/core/src/main/java/org/apache/accumulo/core/conf/ConfigCheckUtil.java b/core/src/main/java/org/apache/accumulo/core/conf/ConfigCheckUtil.java index b9e7ecfff3f..409062dbe24 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/ConfigCheckUtil.java +++ b/core/src/main/java/org/apache/accumulo/core/conf/ConfigCheckUtil.java @@ -23,6 +23,7 @@ import java.util.Objects; import org.apache.accumulo.core.spi.crypto.CryptoServiceFactory; +import org.checkerframework.checker.nullness.qual.NonNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,9 +44,10 @@ public class ConfigCheckUtil { * {@link Property#INSTANCE_ZK_TIMEOUT} within a valid range. * * @param entries iterable through configuration keys and values + * @param source the namespace, table id, site or system config where for diagnostic messages * @throws ConfigCheckException if a fatal configuration error is found */ - public static void validate(Iterable> entries) { + public static void validate(Iterable> entries, @NonNull String source) { String instanceZkTimeoutValue = null; for (Entry entry : entries) { String key = entry.getKey(); @@ -54,12 +56,12 @@ public static void validate(Iterable> entries) { if (prop == null && Property.isValidPropertyKey(key)) { continue; // unknown valid property (i.e. has proper prefix) } else if (prop == null) { - log.warn(PREFIX + "unrecognized property key (" + key + ")"); + log.warn(PREFIX + "unrecognized property key ({}) for {}", key, source); } else if (prop.getType() == PropertyType.PREFIX) { - fatal(PREFIX + "incomplete property key (" + key + ")"); + fatal(PREFIX + "incomplete property key (" + key + ") for " + source); } else if (!prop.getType().isValidFormat(value)) { fatal(PREFIX + "improperly formatted value for key (" + key + ", type=" + prop.getType() - + ") : " + value); + + ") : " + value + " for " + source); } if (key.equals(Property.INSTANCE_ZK_TIMEOUT.getKey())) { @@ -128,7 +130,7 @@ private static void verifyPropertyTypes(PropertyType type, Property... propertie } /** - * The exception thrown when {@link ConfigCheckUtil#validate(Iterable)} fails. + * The exception thrown when {@link ConfigCheckUtil#validate(Iterable, String)} fails. */ public static class ConfigCheckException extends RuntimeException { private static final long serialVersionUID = 1L; diff --git a/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java b/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java index d0e248b2e7f..d965708c939 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java +++ b/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java @@ -209,7 +209,7 @@ public static SiteConfiguration auto() { private final Map config; private SiteConfiguration(Map config) { - ConfigCheckUtil.validate(config.entrySet()); + ConfigCheckUtil.validate(config.entrySet(), "site config"); this.config = config; } diff --git a/core/src/test/java/org/apache/accumulo/core/conf/ConfigCheckUtilTest.java b/core/src/test/java/org/apache/accumulo/core/conf/ConfigCheckUtilTest.java index 258e38075f9..e204316c50f 100644 --- a/core/src/test/java/org/apache/accumulo/core/conf/ConfigCheckUtilTest.java +++ b/core/src/test/java/org/apache/accumulo/core/conf/ConfigCheckUtilTest.java @@ -40,51 +40,51 @@ public void testPass() { m.put(Property.MANAGER_TABLET_BALANCER.getKey(), "org.apache.accumulo.server.manager.balancer.TableLoadBalancer"); m.put(Property.MANAGER_BULK_RETRIES.getKey(), "3"); - ConfigCheckUtil.validate(m.entrySet()); + ConfigCheckUtil.validate(m.entrySet(), "test"); } @Test public void testPass_Empty() { - ConfigCheckUtil.validate(m.entrySet()); + ConfigCheckUtil.validate(m.entrySet(), "test"); } @Test public void testPass_UnrecognizedValidProperty() { m.put(Property.MANAGER_CLIENTPORT.getKey(), "9999"); m.put(Property.MANAGER_PREFIX.getKey() + "something", "abcdefg"); - ConfigCheckUtil.validate(m.entrySet()); + ConfigCheckUtil.validate(m.entrySet(), "test"); } @Test public void testPass_UnrecognizedProperty() { m.put(Property.MANAGER_CLIENTPORT.getKey(), "9999"); m.put("invalid.prefix.value", "abcdefg"); - ConfigCheckUtil.validate(m.entrySet()); + ConfigCheckUtil.validate(m.entrySet(), "test"); } @Test public void testFail_Prefix() { m.put(Property.MANAGER_CLIENTPORT.getKey(), "9999"); m.put(Property.MANAGER_PREFIX.getKey(), "oops"); - assertThrows(ConfigCheckException.class, () -> ConfigCheckUtil.validate(m.entrySet())); + assertThrows(ConfigCheckException.class, () -> ConfigCheckUtil.validate(m.entrySet(), "test")); } @Test public void testFail_InstanceZkTimeoutOutOfRange() { m.put(Property.INSTANCE_ZK_TIMEOUT.getKey(), "10ms"); - assertThrows(ConfigCheckException.class, () -> ConfigCheckUtil.validate(m.entrySet())); + assertThrows(ConfigCheckException.class, () -> ConfigCheckUtil.validate(m.entrySet(), "test")); } @Test public void testFail_badCryptoFactory() { m.put(Property.INSTANCE_CRYPTO_FACTORY.getKey(), "DoesNotExistCryptoFactory"); - assertThrows(ConfigCheckException.class, () -> ConfigCheckUtil.validate(m.entrySet())); + assertThrows(ConfigCheckException.class, () -> ConfigCheckUtil.validate(m.entrySet(), "test")); } @Test public void testPass_defaultCryptoFactory() { m.put(Property.INSTANCE_CRYPTO_FACTORY.getKey(), Property.INSTANCE_CRYPTO_FACTORY.getDefaultValue()); - ConfigCheckUtil.validate(m.entrySet()); + ConfigCheckUtil.validate(m.entrySet(), "test"); } } diff --git a/core/src/test/java/org/apache/accumulo/core/conf/DefaultConfigurationTest.java b/core/src/test/java/org/apache/accumulo/core/conf/DefaultConfigurationTest.java index 123b5bc326e..8ddecfbdf3d 100644 --- a/core/src/test/java/org/apache/accumulo/core/conf/DefaultConfigurationTest.java +++ b/core/src/test/java/org/apache/accumulo/core/conf/DefaultConfigurationTest.java @@ -52,6 +52,6 @@ public void testGetProperties() { @Test public void testSanityCheck() { - ConfigCheckUtil.validate(c); + ConfigCheckUtil.validate(c, "test"); } } diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java b/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java index 287e9bca442..bd2d0b1ace1 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java +++ b/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java @@ -105,7 +105,7 @@ public TableConfiguration getTableConfiguration(TableId tableId) { context.getPropStore().registerAsListener(TablePropKey.of(context, tableId), deleteWatcher); var conf = new TableConfiguration(context, tableId, getNamespaceConfigurationForTable(tableId)); - ConfigCheckUtil.validate(conf); + ConfigCheckUtil.validate(conf, tableId.toString()); return conf; } return null; @@ -129,7 +129,7 @@ public NamespaceConfiguration getNamespaceConfiguration(NamespaceId namespaceId) context.getPropStore().registerAsListener(NamespacePropKey.of(context, namespaceId), deleteWatcher); var conf = new NamespaceConfiguration(context, namespaceId, getSystemConfiguration()); - ConfigCheckUtil.validate(conf); + ConfigCheckUtil.validate(conf, namespaceId.toString()); return conf; }); }